Closed Bug 138877 Opened 22 years ago Closed 22 years ago

Browser crashes if HREF contains entity #9619; M1RC1 topcrash [@ MSVCRT.DLL - PR_Free] [@ libc.so.6 - PR_Free]

Categories

(Core :: Networking: HTTP, defect, P1)

Product:
Core
Component:
Networking: HTTP
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: gashu, Assigned: darin.moz)

References

Details

(4 keywords, Whiteboard: [see also bug 138780] [fixed-trunk])

Crash Data

Attachments

(2 files, 1 obsolete file)

Testcase ( charset: SHIFT-JIS Japanese )
272 bytes, text/html
Details
v2 patch - detects NS_ERROR_UENC_NOMAPPING return codes
1.54 KB, patch
nhottanscp
: review+
rpotts
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.0rc1) Gecko/20020417
BuildID:    2002041711

Browser crashes if HREF contains specific charactors in local charactor set.
It occurs if string length of HREF is continuous 16 or more.


Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.0rc1) Gecko/20020417
WindowsMe JA(Japanese Localized)

Reproducible: Always
Steps to Reproduce:
1. Launch navigator  
2. Open page which includes specific chars in HREF

Actual Results:  Browser crashes.

Expected Results:  Page is displayed.

Source code for testcase as follow:

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Shift_JIS">
<title>TESTCASE</title>
</head>
<body>
<a
href="&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;&#9619;">TEST</a>
</body>
</html>

    
  
Keywords: crash

    
    

    
  
CAUTION: attachment 80224 [details] probably make your moz down. Please make sure your
navigator is ready to crash.

    
    

    
  
Gashu: yes, it does....
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
..am I needing sleep? :)

1.0RC1, Linux -> crash, no Talkback

    
    

    
  
confirming using build 2002041903 on Win2k.
OS -> All.
Talkback ID: TB5425905W.
Keywords: stackwanted, 
          
            testcase
OS: Windows ME → All
Hardware: PC → All

    
    

    
  
Seems, 0.9.8 or older doesn't crash. 0.9.9 or newer does.

    
    

    
  
#0  0x406430c4 in chunk_free (ar_ptr=0x406d7d40, p=0x8716690) at malloc.c:3100
#1  0x40642f9a in __libc_free (mem=0x8716698) at malloc.c:3023
#2  0x40320eba in PR_Free (ptr=0x8716698) at prmem.c:456
#3  0x40252f29 in nsMemoryImpl::Free (this=0x80a2678, ptr=0x8716698)
    at nsMemoryImpl.cpp:342
#4  0x4028d8d2 in nsMemory::Free (ptr=0x8716698) at nsMemory.cpp:95
#5  0x40286621 in nsStrPrivate::Free (aDest=@0x86a9f98) at nsStr.cpp:1104
#6  0x40284060 in nsStrPrivate::Destroy (aDest=@0x86a9f98) at nsStr.cpp:105
#7  0x40286c67 in nsCString::~nsCString (this=0x86a9f94, __in_chrg=2) at
nsString.cpp:103
#8  0x408e2444 in nsStandardURL::~nsStandardURL (this=0x86a9f80, __in_chrg=3)
    at nsStandardURL.cpp:327
#9  0x408e41e2 in nsStandardURL::Release (this=0x86a9f80) at nsStandardURL.cpp:819
#10 0x415d5d15 in nsCOMPtr<nsIURI>::~nsCOMPtr (this=0xbfffe40c, __in_chrg=2)
    at ../../../../dist/include/xpcom/nsCOMPtr.h:490
#11 0x4158d0cb in NS_MakeAbsoluteURIWithCharset (aResult=@0xbfffe488,
aSpec=@0xbfffe4e0, 
    aDocument=0x86baa08, aBaseURI=0x86951f0, aIOService=0x8152de8,
aConvMgr=0x81138b0)
    at nsHTMLUtils.cpp:96
#12 0x412d30e5 in nsHTMLAnchorElement::GetHrefCString (this=0x8716570,
aBuf=@0xbfffe5b8)
    at nsHTMLAnchorElement.cpp:781
#13 0x41592291 in nsStyleUtil::IsHTMLLink (aContent=0x8716570, aTag=0x8181628, 
    aPresContext=0x85454b8, aState=0xbfffe6fc) at nsStyleUtil.cpp:657

Looks like the URI's spec is set to memory it does not own....  To Darin.
Assignee: attinasi → darin
Component: Layout → Networking: HTTP
Keywords: stackwanted
QA Contact: petersen → tever

    
    

    
  
*** Bug 138935 has been marked as a duplicate of this bug. ***

    
    

    
  
i will research this bug.

    
    

    
  
099 can open testcase patch,but when i click the link,it will crash.

    
    

    
  
when mozilla load the testcase patch,it will run 
nsHTMLAnchorElement::GetHrefCString(char* &aBuf),when mozilla want to get the 
relURLspec it will have some error.like that
nsAutoString relURLSpec;

  if (NS_CONTENT_ATTR_HAS_VALUE ==
      NS_STATIC_CAST(nsIContent *, this)->GetAttr(kNameSpaceID_HTML,
                                                  nsHTMLAtoms::href,
                                                  relURLSpec)) { 

    
    

    
  
nsHTMLAnchorElement::GetHrefCString(nsHTMLAnchorElement * const 0x03bbcde8, 
char * & 0x0221daac) line 770
nsStyleUtil::IsHTMLLink(nsIContent * 0x03bbcdb8, nsIAtom * 0x0116b880, 
nsIPresContext * 0x03ba9400, nsLinkState * 0x0012f4cc) line 659
RuleProcessorData::RuleProcessorData(nsIPresContext * 0x03ba9400, nsIContent * 
0x03bbcdb8, nsRuleWalker * 0x03ec9788, nsCompatibility * 0x00000000) line 3308 
+ 29 bytes
ElementRuleProcessorData::ElementRuleProcessorData(nsIPresContext * 0x03ba9400, 
nsIContent * 0x03bbcdb8, nsRuleWalker * 0x03ec9788) line 109
RulesMatchingData::RulesMatchingData(nsIPresContext * 0x03ba9400, nsIAtom * 
0x01145df0, nsIContent * 0x03bbcdb8, nsRuleWalker * 0x03ec9788) line 908 + 27 
bytes
StyleSetImpl::ResolveStyleFor(nsIPresContext * 0x03ba9400, nsIContent * 
0x03bbcdb8, nsIStyleContext * 0x03eed0e0) line 1104
nsPresContext::ResolveStyleContextFor(nsPresContext * const 0x03ba9400, 
nsIContent * 0x03bbcdb8, nsIStyleContext * 0x03eed0e0, nsIStyleContext * * 
0x0012f580) line 972 + 37 bytes
nsCSSFrameConstructor::ResolveStyleContext(nsIPresContext * 0x03ba9400, 
nsIFrame * 0x03eed154, nsIContent * 0x03bbcdb8, nsIStyleContext * * 0x0012f580) 
line 6680 + 29 bytes
nsCSSFrameConstructor::ConstructFrame(nsIPresShell * 0x03e78ce8, nsIPresContext 
* 0x03ba9400, nsFrameConstructorState & {...}, nsIContent * 0x03bbcdb8, 
nsIFrame * 0x03eed154, nsFrameItems & {...}) line 7146 + 44 bytes
nsCSSFrameConstructor::ContentAppended(nsCSSFrameConstructor * const 
0x05a1b778, nsIPresContext * 0x03ba9400, nsIContent * 0x04041b70, int 0) line 
8318
StyleSetImpl::ContentAppended(StyleSetImpl * const 0x03e12190, nsIPresContext * 
0x03ba9400, nsIContent * 0x04041b70, int 0) line 1507
PresShell::ContentAppended(PresShell * const 0x03e78cf0, nsIDocument * 
0x03e85ee8, nsIContent * 0x04041b70, int 0) line 5164 + 49 bytes
nsDocument::ContentAppended(nsDocument * const 0x03e85ee8, nsIContent * 
0x04041b70, int 0) line 1897
nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x03e85ee8, nsIContent * 
0x04041b70, int 0) line 1337 + 17 bytes
HTMLContentSink::NotifyAppend(nsIContent * 0x04041b70, int 0) line 4838
SinkContext::FlushTags(int 1) line 2195
HTMLContentSink::CloseBody(HTMLContentSink * const 0x03e13268, const 
nsIParserNode & {...}) line 3252


    
    

    
  
I think this bug is due to i18n,because i found when 
nsHTMLAnchorElement::GetHrefCString
mozilla will get herf string from attribute,the link in test case should be 
unicode,it's mlength should be 32,but Actual Results is 16,when mozilla run
 NS_MakeAbsoluteURIWithCharset(buf, relURLSpec, mDocument, baseURL,
                                    nsHTMLUtils::IOService,
                                    nsHTMLUtils::CharsetMgr);
i will new a uri object to get a new url string,but when mozilla free the uri 
object and free uri's mspec,it will some error happen.

    
    

    
  
Possibly, this bug depends on bug 100676 and bug 129279 - and those may relates
bug 125106.

Assuming: 
1. common unicode conversion is doing bogus thing.
OR
2. Missing conversion procedure somewhere.
OR
3. Unneccesarry conversion procedure somewhre. 


...I've got a bad idea. this bug means anyone can create a page that knocks moz
down so easily. plus, can do it via local web forums...

    
    

    
  
I can confirm this bug. It bit me at Slashdot today.

The Slashdotter paolo goblino uses a href like this
in his sig - possibly with the intention to bring 
Mozilla down. Here's comment from him that crashes 
Mozilla:

<http://slashdot.org/comments.pl?sid=31489&cid=3388760>

It seems this bug does not bite if "utf-8" is declared.

Here are two simple test pages. They are identical, except
that the second one has a meta-element that declares the
encoding "utf-8":

  <http://www.bertilow.com/div/dark_shade/might_crash.html
  <http://www.bertilow.com/div/dark_shade/will_not_crash.html>

As it seems this bug is already being exploited to crash
Mozilla. So it is a major problem.

    
    

    
  
Nominating for 1.0.  This is getting some visibility and people are using it on
discussion boards to bring Mozilla down.  Seen this already on Slashdot and Plastic.

(Also I think the charset thing is a red herring.  Testcases work regardless of
charset.)
Keywords: mozilla1.0

    
    

    
  
*** Bug 139273 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 139266 has been marked as a duplicate of this bug. ***

    
    

    
  
One of the comments in the slashdot story about MS pushing licences is crashing
a lot of people using this bug.
The guy has a signature that has a href with unicode(?) characters.
(See the bug I just duped. It already had so many dupes so fast.)

    
  
Summary: Browser crashes if HREF contains specific charactors in local charactor set. → Browser crashes if HREF contains specific characters in local character set.

    
    

    
  
Removing misleading extraneous info from Summary.
Summary: Browser crashes if HREF contains specific characters in local character set. → Browser crashes if HREF contains entity #9619

    
    

    
  
*** Bug 139277 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 139274 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 139269 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 139282 has been marked as a duplicate of this bug. ***

    
  
Keywords: topcrash

    
  
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.0

    
  
Keywords: nsbeta1, 
          
            topembed

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v1 patch (obsolete)
        — 
          — Splinter Review
      

    


  
ok, the problem is in nsStandardURL.cpp:EncodeString().  nsStandardURL tries to
convert the URL from unicode to Shift-JIS.  the conversion fails, presumably
because the characters cannot be represented in Shift-JIS (perhaps some i18n
expert can verify this fact).  the crash was caused by the fact that
nsStandardURL was not properly handling the conversion failure.  (the Shift-JIS
converter does not throw an exception; it just converts zero bytes.)  as a
result, we were overwriting the array boundaries of nsStandardURL::mSpec, so
it's no wonder that free'ing mSpec resulted in a SEGV.

the solution is to detect the conversion failure, and simply construct the URL
using UTF-8 instead of Shift-JIS.

with this patch, i'm able to load the testcase.  hovering over the "TEST" link
reveals what appear to be chinese characters to my untrained eye.  can these be
represented using Shift-JIS?  anyways, clicking on the link fails (obviously
since bugzilla.m.o does not have the resulting page).  the 404 message confirms
that the URL sent to the server is encoded using UTF-8.

can someone verify that this is the correct way of handling the conversion
failure?

    
    

    
  
I think this patch can solve crash bug,but it will make mozilla can not open 
this page,do you think we can deal with this problem as utf8,like this page's 
logic 
<http://www.bertilow.com/div/dark_shade/will_not_crash.html>


    
    

    
  
FYI:
#9619 is one of the Block Elements in unicode.
http://www.hclrss.demon.co.uk/unicode/block_elements.html

I confirmed it also bites using #9618.
Perhaps, also #9600-#9631 and...so far.

It seems, conversion for specific UTF address fails.


see... this patch is a temporary measure and our critical mission is to detect
where the devil we don't know - the conversion failure at first.  Is my
understanding right? 

I think if deal with it as UTF8, it can make another problem like font aliasing
(e.g ASCII code in SHIFT-JIS use Japanese font, but in UTF8 use Western font.). 

    
    

    
  
> e.g ASCII code in SHIFT-JIS use Japanese font, but in UTF8 use Western font.

Correction for above.. It is IE6's case. Mozilla uses Japanese font in UTF8.

    
  
Whiteboard: [see also bug 138780]

    
    

    
  
I think fallback to UTF-8 is fine because there's no code point in Shift_JIS for
&#9619; (\u2593).
The string converted here is URI in HREF. I don't think display issue involving
here.


    
    

    
  
Comment on attachment 80546 [details] [diff] [review]
v1 patch

The conversion result could contain fewer characters, for example UCS2 to
ISO-8859-1 which converts 2 bytes to 1 bytes.
I think we can add error checks. 
* check the error code NS_ERROR_UENC_NOMAPPING after Convert().
* check 'len' if zero after Convert() call (but not for Finish()).

        Attachment #80546 -
        Flags: needs-work+

    
    

    
  
*** Bug 123316 has been marked as a duplicate of this bug. ***

    
    

    
  
nhotta: fewer bytes, but not fewer "storage units"... 

  NS_LITERAL_CSTRING("foo").Length() == NS_LITERAL_STRING("foo").Length()

ah, so my NS_FAILED(rv) checks are not being hit because NS_ERROR_UENC_NOMAPPING
is a success code.  that bites! ;-)

i'll go ahead and put together a patch that checks for NS_ERROR_UENC_NOMAPPING, thx!

    
    

    
  
It is nsACString, result.Length() this actually returns number of characters
only if 1 character is 1 byte. If 1 character is 2 bytes or more then returned
length is a byte length. 
E.g. 0xE38182, this is one character in UTF-8 but nsACString::Length() returns 3.



    
    

    
  
nhotta: right... that's why i said "storage units" instead of "characters" ;-)

anyways, i can remove the Length() comparison since Convert() does indeed return
a "success" error code... what an oxymoron! :-)

    
    

    
  
Adding M1RC1 topcrash [@ MSVCRT.DLL - PR_Free] to summary for tracking.
Summary: Browser crashes if HREF contains entity #9619 → Browser crashes if HREF contains entity #9619; M1RC1 topcrash [@ MSVCRT.DLL - PR_Free]

    
    

    
  


  

        Attachment #80546 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 81072 [details] [diff] [review]
v2 patch - detects NS_ERROR_UENC_NOMAPPING return codes

r=nhotta

        Attachment #81072 -
        Flags: review+

    
  
Keywords: adt1.0.0

    
    

    
  
Darin, could you get a sr= for this.  I'd like to consider this for beta.
Whiteboard: [see also bug 138780] → [see also bug 138780] [needs sr=]

    
    

    
  
Comment on attachment 81072 [details] [diff] [review]
v2 patch - detects NS_ERROR_UENC_NOMAPPING return codes

sr=rpotts@netscape.com

looks good.  is there any posibility that encoder->Reset() would fail when
everything else had succeeded?

cause in that case, we'll be returning NS_OK where we used to return
NS_ERROR_XXX.

i doubt if it's a big deal -- just asking :-)

-- rick

        Attachment #81072 -
        Flags: superreview+

    
    

    
  
Adding [@ libc.so.6 - PR_Free] to summary for tracking, the majority of crashes
on Linux are being reported under that stack signature.
Summary: Browser crashes if HREF contains entity #9619; M1RC1 topcrash [@ MSVCRT.DLL - PR_Free] → Browser crashes if HREF contains entity #9619; M1RC1 topcrash [@ MSVCRT.DLL - PR_Free] [@ libc.so.6 - PR_Free]

    
    

    
  
fixed-on-trunk
Whiteboard: [see also bug 138780] [needs sr=] → [see also bug 138780] [fixed-trunk]

    
    

    
  
Comment on attachment 81072 [details] [diff] [review]
v2 patch - detects NS_ERROR_UENC_NOMAPPING return codes

a=asa (on behalf of drivers) for checkin to the 1.0 branch

        Attachment #81072 -
        Flags: approval+

    
    

    
  
adding adt1.0.0+. Please check this into the branch as soon as possible and add
the fixed1.0.0 keyword.
Keywords: adt1.0.0 → adt1.0.0+

    
    

    
  
fixed-on-branch
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED

    
    

    
  
*** Bug 132657 has been marked as a duplicate of this bug. ***

    
    

    
  
verified using testcase in comment #2 - 04/02/02 trunk and branch, winNT4, Linux
rh6, mac osX
Status: RESOLVED → VERIFIED

    
  
Keywords: verified1.0.0

    
    

    
  
forgot to remove fixed1.0.0 keyword so doing so now
Keywords: fixed1.0.0
Crash Signature: [@ MSVCRT.DLL - PR_Free]
[@ libc.so.6 - PR_Free]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: