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)

ARM
Gonk (Firefox OS)
defect
Not set
critical

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)

VERIFIED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
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)

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
attach file to bugzilla failed

The attachment is a 0 Byte file, its name is .nomedia
Please provide a crash ID (see https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Getting_crashes_off_the_Device).
Severity: normal → critical
Keywords: crash, stackwanted
Whiteboard: [b2g-crash]
Attached file adb log
Provide adb log in attachment
Could you check if this is this related with 875408?? Thanks!
Flags: needinfo?(pyang)
Summary: [MMS] Receive unsupported file and crash when entering thread view → [MMS] Receive zero size file will crash when broadcast the dom message.
Broja, 
I think it is unrelated. This bug is for the file which is zero size.
Flags: needinfo?(pyang)
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
blocking-b2g: --- → leo?
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)
Attachment #764078 - Flags: review?(gene.lian)
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.
Triage - as easily reproduced this could be a potential attack on all firefox phones. Blocking.
blocking-b2g: leo? → leo+
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+
(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.
Backed out for no reviewer on patch comment. https://hg.mozilla.org/projects/birch/rev/0664d639c478
https://hg.mozilla.org/mozilla-central/rev/1b659b9958cb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [b2g-crash] → [b2g-crash] leorun3
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.

Attachment

General

Created:
Updated:
Size: