Firefox3.1/SMIL Security Review

From MozillaWiki
Jump to: navigation, search

Overview

Describe the goals and objectives of the feature here.

Background links

Security and Privacy

  • What security issues do you address in your project?
    • None.
  • Is system or subsystem security compromised in any way if your project's configuration files / prefs are corrupt or missing?
    • No. There are no configuration files, and the only planned about:config pref is just an enable/disable flag. (see pref description below)
  • Include a thorough description of the security assumptions, capabilities and any potential risks (possible attack points) being introduced by your project.
    • Security Assumptions:
      • SVG documents aren't allowed to draw outside of their boundaries, or over Firefox chrome.
    • Potential attack points:
      • If embedded SVG were allowed to draw outside of its boundaries, it could use SMIL to cover bits of the host document with its own SVG elements.
      • Integer overflows in SMIL attributes, or in animated SVG attributes, or in other attributes that depend on the animated attributes (e.g. heights/widths during a skew animated with animateTransform )

Exported APIs

  • Does it interoperate with a web service? How will it do so?
    • No.
  • Explain the significant file formats, names, syntax, and semantics.
    • Main file format: SVG
    • Sample animation tag usage:
<rect x="15" y="200" width="200" height="200" fill="blue">
   <animate attributeName="y" from="100" to="200" begin="0s" dur="2s" fill="freeze"/>
</rect>
 <rect x="15" y="200" width="200" height="200" fill="blue">
   <animateTransform attributeName="transform" type="rotate"
                     from="-30" to="0"
                     begin="1s" dur="5s" fill="freeze"/> 
 </rect>
<circle cx="0" cy="0" stroke="black" r=".05" fill="orange" stroke-width="0.005">
   <animate attributeName="cy" dur="3s" values="0; 1"
            calcMode="spline" keySplines=".5 0 .5 1"
            repeatDur="indefinite" />
   <animate attributeName="cx" dur="3s" values="0; 1"
            repeatDur="indefinite" />
</circle>
  • Are the externally visible interfaces documented clearly enough for a non-Mozilla developer to use them successfully?
    • They're documented in the SVG & SMIL Animation specs.
    • We could probably use up-to-date documentation on MDC detailing what we support.
  • Does it change any existing interfaces?
    • It adds implementations for these methods on nsIDOMSVGSVGElement.idl, which previously all just returned NS_NOTYETIMPLEMENTED:
      • void pauseAnimations()
      • void unpauseAnimations()
      • boolean animationsPaused()
      • float getCurrentTime()
      • void setCurrentTime(in float seconds)

Module interactions

  • What other modules are used (REQUIRES in the makefile, interfaces)
    • gfx, layout, content, dom

Data

  • What data is read or parsed by this feature
    • The values of the SVG <animate>, <set>, and <animateTransform> elements' attributes. These consist of semicolon-separated lists of values, time values, special string values (e.g. "indefinite")
    • The SMIL patch adds the class "nsSMILParserUtils," which takes care of its parsing needs.
  • What is the output of this feature
    • Animations (animated attributes) in SVG documents that would otherwise be static.
  • What storage formats are used
    • nsSMILValue stores the actual animated attribute values
    • Other containers: nsTArray, nsCOMArray

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?
    • N/A -- no files or locks used.

Configuration

  • Can the end user configure settings, via a UI or about:config? Hidden prefs? Environment variables?
    • Yes: I'm planning on using an about:config pref to allow the user to enable/disable SMIL. (Either using the existing image.animation_mode pref, or adding a smil-specific pref)
  • Are there build options for developers? [#ifdefs, ac_add_options, etc.]
    • Yes: "ac_add_options --enable-smil". (The current patch defaults to having SMIL support disabled.)
  • What ranges for the tunable are appropriate? How are they determined?
    • Enabled / Disabled
    • One possible additional about:config value: I had a request in IRC for the about:config pref to have a value that would show animations *only* in standalone SVG but not in SVG-embedded-in-HTML, for performance reasons. Haven't given this much thought yet, and I'm not sure how big of a use case it is.
      • This additional value would mean we'd need the about:config pref to be SMIL-specific (not the shared image.animations_enabled one), and would need to be string-valued rather than bool-valued.
  • What are its on-going maintenance requirements (e.g. Web links, perishable data files)?
    • N/A

Relationships to other projects

  • Are there related projects in the community?
    • Opera and WebKit both support subsets of SMIL. FakeSMIL is a Greasemonkey script that uses Javascript to add partial SMIL support to Firefox.
  • If so, what is the proposal's relationship to their work? Do you depend on others' work, or vice-versa?
    • This proposal is parallel to their work, adding built-in SMIL support to Firefox.
  • 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?
    • No & N/A -- I'm not updating, copying or changing functional areas maintained by other groups.

Review comments

  • need a SMIL-enabled pref, separate from image animation.
    • is it worth checking both before doing animation? For security and stability reasons we may want to be able to kill SMIL, but users who just want to stop animated ads may expect the old pref to stop all kinds of animation.
  • only support length values for animation
    • spec supports any CSS property
  • only times relative to the beginning of the document (spec has a richer set): begin, end, dur(ation).
  • times are stored as PRInt64
  • need to test negative durations and end times prior to begin times
  • can specify lists of begin and end times
    • what if they don't have the same number of elements?
      • dholbert: I misspoke about this during the security review. What actually happens is this:
        • An animation needs one or more "begin" values and exactly one "dur" value.
        • As time passes, we'll (re)start the animation each time we hit a "begin" value.
        • If there's an "end" list, then each time we cross a time in the "end" list, we interrupt the current interval of the animation (if it's playing). (Upon interrupting, we either freeze the animation or remove its effects, depending on the value of the "fill" attribute)
        • Moreover, the largest "end" value defines a "final" end, at which point any subsequent "begin" values will not have any effect. (Note that you can get around this by putting the special string "indefinite" in the end list -- this value is never reached in time.)
      • So, the number of "begin" vs. number of "end" elements doesn't really matter. Hopefully this is more clear.
    • what if they are misordered? Do they match by index in the given lists or do the begins run until the next specified end?
      • dholbert: List-order doesn't matter in the begin / end lists -- only temporal order. Basically, each "begin" triggers an instance of the animation (with duration "dur"), and the run will be interrupted by any "end" times encountered during it run. (See answer to previous question for more detail)
  • integer overflows if images get too big? translated way offscreen?
    • SVG keeps things in floating point, cairo's interface is floating point
    • cairo does the conversion into actual images
  • spec says there should be animation events, but we don't fire them. Should we ever do that we would need to be careful.
  • animation nodes can be moved from element to element -- test
  • tests
    • calcModes: discrete, spline, paced, linear
    • "fill" values: freeze, remove?
    • closing the window right after setting current time
    • destroy iframe while animating
    • navigate to a new page while animating
    • delete animating element; move it elsewhere in the document
  • need to make sure these elements and attributes will be added by the SVG fuzzer