Closed
Bug 549389
Opened 14 years ago
Closed 14 years ago
Regression: String operations on a string that ends with a partial surrogate character can result in a crash
Categories
(Tamarin Graveyard :: Virtual Machine, defect, P2)
Tamarin Graveyard
Virtual Machine
Tracking
(Not tracked)
VERIFIED
FIXED
flash10.1
People
(Reporter: wsharp, Assigned: wsharp)
Details
(Whiteboard: Has patch)
Attachments
(2 files)
1.23 KB,
patch
|
stejohns
:
review+
|
Details | Diff | Splinter Review |
2.33 KB,
patch
|
dschaffe
:
review+
|
Details | Diff | Splinter Review |
Constructing a StUTF8String object from a UTF-16 character that ends in a partial surrogate character will result in a call to GC::Alloc with a size of zero. This will crash on release builds. This is a regression -- crashes in 10.1 but not in 10.0.45. At line 2697 of StUTF8String::StUTF8String, when it calls UnicodeUtils::Utf16ToUtf8, it does not handle the case where UnicodeUtils::Utf16ToUtf8 returns -1. It will do so if the str parameter ends with half of a surrogate pair. Partial surrogate pairs may be produced when deleting characters. See bug 2522195. Bug Group bug 2562110 contains a simple test app that will allow this bug to be reproduced. It also has instructions on how to enter surrogate characters and generate the right conditions for this bug to occur. See also sample code below for a programmatic crasher case. var s:String = "abc"; var badChar:String = String.fromCharCode(0xD801); s += badChar; s.replace(/\uD801/, "\\uD801"); player bug #2564537
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → wsharp
OS: Windows Vista → All
Hardware: x86 → All
Target Milestone: --- → flash10.1
Comment 1•14 years ago
|
||
(In reply to comment #0) > Constructing a StUTF8String object from a UTF-16 character that ends in a > partial surrogate character will result in a call to GC::Alloc with a size of > zero. This will crash on release builds. I have suspected for some time that making zero-size allocations illegal - which is an explicit choice - is probably an API bug in MMgc, but I don't know if we want to fix that API right now or just work around this problem.
Assignee | ||
Comment 2•14 years ago
|
||
Prior implementation just converted the -1 length to 0. if( utf8len < 0 ) { utf8len = 0; } I was just planning to do the same code in the current code: len = UnicodeUtils::Utf16ToUtf8(String::Pointers(str).p16, str->length(), NULL, 0); if (len < 0) len = 0; char* dstBuf = (char*) gc->Alloc(uint32_t(len)+1, 0);
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #429831 -
Flags: review?(stejohns)
Updated•14 years ago
|
Attachment #429831 -
Flags: review?(stejohns) → review+
Comment 4•14 years ago
|
||
Comment on attachment 429831 [details] [diff] [review] len < 0 patch Have we checked all the other calls to Utf16toUtf8 to see if they also check for negative results?
Assignee | ||
Comment 5•14 years ago
|
||
This is the only case. We call it to get the length then we call it again with our buffer.
Comment 6•14 years ago
|
||
can this land?
Comment 7•14 years ago
|
||
tamarin-redux changeset 4015:03d093d2cf20 tr-argo changeset 3814:b28ad909f055
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 8•14 years ago
|
||
Attachment #433580 -
Flags: review?(dschaffe)
Updated•14 years ago
|
Attachment #433580 -
Flags: review?(dschaffe) → review+
Comment 9•14 years ago
|
||
Comment on attachment 433580 [details] [diff] [review] testcase testcase pushed tr-argo -> 3873:8265b24fbef7 tr -> 4179:8265b24fbef7
Updated•14 years ago
|
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Flags: flashplayer-triage+
You need to log in
before you can comment on or make changes to this bug.
Description
•