Closed Bug 138877 Opened 23 years ago Closed 23 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: 23 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: