Confirmed users
489
edits
Firefox3.1/SMIL Security Review: Difference between revisions
Firefox3.1/SMIL Security Review (view source)
Revision as of 02:31, 11 September 2008
, 11 September 2008→Exported APIs: - switch ElementTimeControl interface to the one defined in SVG spec (better documented) rather than the one in the smil-animation spec
(→Exported APIs: - switch ElementTimeControl interface to the one defined in SVG spec (better documented) rather than the one in the smil-animation spec) |
|||
| (12 intermediate revisions by 2 users not shown) | |||
| Line 4: | Line 4: | ||
;Background links | ;Background links | ||
* [https://bugzilla.mozilla.org/show_bug.cgi?id=216462 Bug 216462: Main SMIL bug] | * [https://bugzilla.mozilla.org/show_bug.cgi?id=216462 Bug 216462: Main SMIL bug] | ||
** [https://bugzilla.mozilla.org/attachment.cgi?id=336084&action=diff Latest patch] | |||
* [http://www.w3.org/TR/SVG11/animate.html SVG 1.1 Spec's Animation Section] | * [http://www.w3.org/TR/SVG11/animate.html SVG 1.1 Spec's Animation Section] | ||
* [http://www.w3.org/TR/smil-animation SMIL Animation Spec] | * [http://www.w3.org/TR/smil-animation SMIL Animation Spec] | ||
| Line 16: | Line 17: | ||
* Include a thorough description of the security assumptions, capabilities and any potential risks (possible attack points) being introduced by your project. | * 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 == | == Exported APIs == | ||
* Please provide a table of exported interfaces (APIs, ABIs, protocols, UI, etc.) | * Please provide a table of exported interfaces (APIs, ABIs, protocols, UI, etc.) | ||
** [http://www.w3.org/TR/ | ** [http://www.w3.org/TR/SVG11/animate.html#InterfaceElementTimeControl nsIDOMElementTimeControl.idl] | ||
*** boolean beginElement() | *** boolean beginElement() | ||
*** boolean beginElementAt(in float offset) | *** boolean beginElementAt(in float offset) | ||
| Line 35: | Line 41: | ||
* Explain the significant file formats, names, syntax, and semantics. | * 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? | * Are the externally visible interfaces documented clearly enough for a non-Mozilla developer to use them successfully? | ||
| Line 55: | Line 79: | ||
== Data == | == Data == | ||
* What data is read or parsed by this feature | * 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, .. | ** 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. | ** The SMIL patch adds the class "nsSMILParserUtils," which takes care of its parsing needs. | ||
| Line 62: | Line 86: | ||
* What storage formats are used | * What storage formats are used | ||
** nsSMILValue stores the actual animated attribute values | |||
** Other containers: nsTArray, nsCOMArray | |||
== Reliability == | == Reliability == | ||
| Line 96: | Line 122: | ||
== Review comments == | == 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? | |||
*** <b>dholbert</b>: 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? | |||
*** <b>dholbert</b>: 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 | |||