Firefox3.7/about:crashes Security Review

From MozillaWiki
Jump to: navigation, search

Overview

I have added the ability to submit crash reports from the "pending" folder via the about:crashes page. This can include crashes that failed to send for some reason, or crashes that were intentionally throttled by the server. This is important because we would like to use the server throttling, but clients previously had no way to resubmit crashes.

This document is still in progress

Background links

Security and Privacy

  • Is this feature a security feature?
    • No
  • What potential security issues in your feature have you already considered and addressed?
    • The feature relies on submitting a form via HTTP POST, and it does so by creating a <xul:iframe> as a child of the about: document. The form is submitted to a URL that is associated with the crash report, which is stored in application.ini, and is a HTTPS URL for all our shipping products.
  • Is system or subsystem security compromised in any way if your project's configuration files / prefs are corrupt or missing?
    • about:crashes page will display an error message if the server URL pref is missing
    • Corrupted data in the extra data file would be ignored, or non-fatal at worst (crash report might be ignored by server for missing data)
  • Include a thorough description of the security assumptions, capabilities and any potential risks (possible attack points) being introduced by your project.
  • How are transitions in/out of Private Browsing mode handled?
    • N/A

Exported APIs

  • Please provide a table of exported interfaces (APIs, ABIs, protocols, UI, etc.)
    • None
  • Does it interoperate with a web service? How will it do so?
  • Explain the significant file formats, names, syntax, and semantics.
    • Reads additional crash data as key=value pairs in a text file
  • Are the externally visible interfaces documented clearly enough for a non-Mozilla developer to use them successfully?
    • N/A
  • Does it change any existing interfaces?
    • N/A

Module interactions

  • What other modules are used (REQUIRES in the makefile, interfaces)?
    • HTTP POST using a html <form>

Data

  • What data is read or parsed by this feature?
    • Key/value pairs from plain text file
    • Key/value pairs from HTTP response body
  • What is the output of this feature?
    • Writes a text file in the Crash Reports/submitted folder
  • What storage formats are used?
    • Plain text key=value files, plain text for the submit record

Reliability

  • What failure modes or decision points are presented to the user?
    • None
  • Can its files be corrupted by failures? Does it clean up any locks/files after crashes?
    • No locks, failure would simply cause the report to not be submitted

Configuration

  • Can the end user configure settings, via a UI or about:config? Hidden prefs? Environment variables?
    • N/A
  • Are there build options for developers? [#ifdefs, ac_add_options, etc.]
    • Contained within the existing --disable-crashreporter
  • What ranges for the tunable are appropriate? How are they determined?
    • N/A
  • What are its on-going maintenance requirements (e.g. Web links, perishable data files)?

Relationships to other projects

Are there related projects in the community?

  • Not AFAIK
  • If so, what is the proposal's relationship to their work? Do you depend on others' work, or vice-versa?
  • Are you updating, copying or changing functional areas maintained by other groups? How are you coordinating and communicating with them? Do they "approve" of what you propose?

Review comments

  • the iframe needs to be type="content" to make sure the results don't get chrome privileges - filed bug 512853
  • pending crash data isn't used on the about:crashes page, just the name of the crash file. It's added to the page as a text node -- no chance to escape.
  • If you crash in private browsing mode do we send the URL you crashed on? Since crash-stats doesn't show the URL it probably doesn't matter.
  • the number-of-occurrences version of split() truncates, this might be a dataloss bug but not a security problem with the submitted pending crash data (might happen in comments with an equal sign, for example) - also in patch on bug 512853