Closed Bug 1390547 Opened 4 years ago Closed 3 years ago

Strip newlines from MozCrashReason annotation


(Toolkit :: Crash Reporting, enhancement)

Not set



Tracking Status
firefox64 --- fixed


(Reporter: kats, Assigned: gsvelto)




(1 file)

The code at [1] writes the MozCrashReason annotation to crash reports. However this crash reason can be pretty much arbitrary data (specially now that we've wired up rust panic messages to it).

If that message contains newlines, it can mess up the crash report submission because the report syntax is newline-delimited key=value pairs. In fact, by calling something like this:
in the code it's possible to generate a crash report with the key=value pair showing up as a separate entry in the metadata table of the crash report [2]. It might be possible to overwrite other keys in the crash report or generate invalid data such that the report itself gets dropped.

While this isn't particularly harmful it might result in bad crashdata, and anway it's kind of unexpected and probably something worth guarding against.

I didn't check the other annotation keys to see if their values can also end up with newlines injected into them; MozCrashReason might not be the only one.

[2] note the "secondline" key i added with this technique. changeset is at
This is pretty bad actually, we already had other problems WRT this, see bug 1395507 for a particularly egregious one. This is probably worth fixing but as Ted already mentioned multiple times we should really drop the INI-style format for the .extra file and just write out a JSON blob.
We escape newlines and backslashes in annotations added using `AnnotateCrashReport`:

`MozCrashReason` gets special handling so it misses all that, unfortunately.
Assignee: nobody → gsvelto
The original patch did not fix the issue with content crashes. I've uploaded a new version that fixes the issue in content processes and introduces unit-tests covering both main and content process crashes.
Review ping
Flags: needinfo?(ted)
Pushed by
Escape MozCrashReason when writing out a crash report r=ted
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: needinfo?(ted)
You need to log in before you can comment on or make changes to this bug.