Closed
Bug 883296
Opened 11 years ago
Closed 11 years ago
[MMS] Receive zero size file will crash when broadcast the dom message.
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)
People
(Reporter: pyang, Assigned: kk1fff)
References
Details
(Keywords: crash, stackwanted, Whiteboard: [b2g-crash] leorun3)
Attachments
(2 files, 1 obsolete file)
387.44 KB,
text/x-vhdl
|
Details | |
3.96 KB,
patch
|
airpingu
:
review+
|
Details | Diff | Splinter Review |
build: v1-train pvt build 2013061007 device: Leo Prerequisite: Use attached file, sent MMS with an android or other phone. Auto-retrieve enabled (not required) REPRODUCE STEP: 1. Send MMS with attachment (a unsupported file) 2. Click notification EXPECT: An unsupported preview icon is shown ACTUAL: B2G crashed while entering thread view
Reporter | ||
Comment 1•11 years ago
|
||
attach file to bugzilla failed The attachment is a 0 Byte file, its name is .nomedia
Comment 2•11 years ago
|
||
Please provide a crash ID (see https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Getting_crashes_off_the_Device).
Reporter | ||
Comment 3•11 years ago
|
||
Provide adb log in attachment
Comment 4•11 years ago
|
||
Could you check if this is this related with 875408?? Thanks!
Flags: needinfo?(pyang)
Updated•11 years ago
|
Summary: [MMS] Receive unsupported file and crash when entering thread view → [MMS] Receive zero size file will crash when broadcast the dom message.
Comment 5•11 years ago
|
||
Broja, I think it is unrelated. This bug is for the file which is zero size.
Flags: needinfo?(pyang)
Assignee | ||
Comment 6•11 years ago
|
||
When receiving a zero length file, content of the attachment structure will be null (instead of a zero length blob). This crashes main process when trying to form an MmsMessageData for an IPC operation. We should store a zero length blob for zero length attachment instead of using null pointer.
Assignee: nobody → pwang
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #764029 -
Flags: review?(gene.lian)
Updated•11 years ago
|
blocking-b2g: --- → leo?
Assignee | ||
Comment 8•11 years ago
|
||
We should return an empty array instead of an empty object. try: https://tbpl.mozilla.org/?tree=Try&rev=6e90a0f7c8e3
Attachment #764029 -
Attachment is obsolete: true
Attachment #764029 -
Flags: review?(gene.lian)
Assignee | ||
Updated•11 years ago
|
Attachment #764078 -
Flags: review?(gene.lian)
Comment 9•11 years ago
|
||
Comment on attachment 764078 [details] [diff] [review] Patch: returning empty array instead of null when decoding mms attachment Review of attachment 764078 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/src/ril/WspPduHelper.jsm @@ -2291,4 @@ > > let octetArray = Octet.decodeMultiple(data, contentEnd); > let content = null; > - if (octetArray) { We can still have null octetArray here. Please keep it until we have better error handling for these broken blobs.
Comment 10•11 years ago
|
||
Triage - as easily reproduced this could be a potential attack on all firefox phones. Blocking.
blocking-b2g: leo? → leo+
Comment 11•11 years ago
|
||
Comment on attachment 764078 [details] [diff] [review] Patch: returning empty array instead of null when decoding mms attachment Review of attachment 764078 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #764078 -
Flags: review?(gene.lian) → review+
Comment 12•11 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #9) > We can still have null octetArray here. Please keep it until we have better > error handling for these broken blobs. After some more discuss, |Octet.decodeMultiple| can no longer return null after this change, so checking nullity of its return value is meaningless. I concerned about a conformance requirement saying that we should do our best effort to handle broken PDU and return as much information as possible. Even so, we should catch the possible exception here and create an empty blob instead. But that's off the topic here.
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/9a3ee53f3421
Assignee | ||
Comment 14•11 years ago
|
||
Backed out for no reviewer on patch comment. https://hg.mozilla.org/projects/birch/rev/0664d639c478
Assignee | ||
Comment 15•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/1b659b9958cb
Comment 16•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1b659b9958cb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-firefox24:
--- → fixed
Updated•11 years ago
|
Whiteboard: [b2g-crash] → [b2g-crash] leorun3
Comment 17•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/8be1c897c934
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-b2g-v1.1hd:
--- → affected
status-firefox22:
--- → wontfix
status-firefox23:
--- → wontfix
Target Milestone: --- → 1.1 QE3 (24jun)
Reporter | ||
Comment 20•11 years ago
|
||
Verified by pvt build 20130715230201 Issue resolved without crash
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•