Open GoogleCodeExporter opened 9 years ago
Original comment by grick23@gmail.com
on 3 Sep 2009 at 8:28
Original comment by bradneub...@gmail.com
on 20 Oct 2009 at 12:34
I have added something approximating the suggestion by Rick. Heres the comment
I added to svg.js above the _forceFlash function and the two new supporting
functions I added as part of the patch:
/** The following _forceFlash functions determine whether to force
the flash renderer or not. The forceflash parameter can be set
using either META tags or query string arguments are supported, with
query string arguments having the higher priority.
Parameters accepted:
svg.render.forceflash
If the value is the string 'true' regardless of case, the default
setting for forceflash is set to true, otherwise it is set to false.
example: <meta name="svg.render.forceflash" content="true"/>
svg.render.forceflash.true
The value consists of an optional property name of window
followed by a '|' followed by a mandatory regular expression.
If the property name is not specified, it is assumed to be
'navigator.userAgent'. If the value of the property is tested
against the regular expression and is matched, then the specific
setting for forceflash is set to true, otherwise it is not changed.
example: <meta name="svg.render.forceflash.true" content="navigator.userAgent|MSIE [678]"/>
svg.render.forceflash.false
The value consists of an optional property name of window
followed by a '|' followed by a mandatory regular expression.
If the property name is not specified, it is assumed to be
'navigator.userAgent'. If the value of the property is tested
against the regular expression and is matched, then the specific
setting for forceflash is set to false, otherwise it is not changed.
example: <meta name="svg.render.forceflash.false" content="navigator.userAgent|Chrome"/>
If a specific setting exists (from a svg.render.forceflash.true or
svg.render.forceflash.false parameter), it will be used as the return
value for _forceFlash. If no specific setting exists, then the default
setting will be used as the return value for _forceFlash.
*/
If the page included svg.render.forceflash set to true, that would be what I've
termed the default setting. If the page also included a
svg.render.forceflash.true parameter and the property/regular expression in its
value evaluates to true in the current browser, then whats I've termed the
specific setting, and it would take precedence over the default setting, so the
browser would have the flash renderer forced on.
There is the potential for people to use this to check more than just
navigator.userAgent, though that is the default property it checks. It starts
at window and walks its way down to the specific property mentioned in the
parameter. So things like the document.location.search could be checked, and
so forth.
Please note that the diff attached has the changes from Issue 368 at the very
bottom and can be ignored as they do not relate to this Issue.
Original comment by bduncan%...@gtempaccount.com
on 26 Jul 2010 at 3:33
Attachments:
Hi Bruce, thanks for making this. I'm worried about allowing arbitrary
JavaScript in the URL which can open XSS attacks. I'd rather restrict it to
just being userAgent matching, or perhaps have a short string of actual
browsers which matches the internal browser checking already present in SVG
Web, such as isIE, isIE8, isGecko, etc. You should whitelist on these rather
than making them dynamic again to protect against XSS attacks.
Whenever things are passed in the URL, especially for a generalized library
like SVG Web, I like to be extra careful of XSS attacks.
Original comment by bradneub...@gmail.com
on 26 Jul 2010 at 8:29
I understand the concern regarding XSS, but I'm not sure I understand how my
code would allow for arbitrary code execution. It does check for a property
that can be specified on the query string, that is true. And it does run a
regexp by doing "new RegExp(STRING FROM QUERY STRING OR META TAG).test(VALUE OF
PROPERTY)", but I dont use "eval" or any other function that would allow for
arbitrary code execution.
I looked at the browser checking code in svgweb before beginning this, and
didnt see any consistency there. Perhaps if that code would produce a browser
name and version in strings that could be checked I would have simply
implemented it as you suggested, by allowing people to specify things based on
a list of browser names and/or versions. But thats currently not the case.
And when I saw that the code for detecting some browsers used
navigator.userAgent while code for checking for other browsers used
navigator.appVersion, etc, it seemed to me more flexible to allow them to check
against whichever property made the most sense for what it was they were trying
to check against (though most of the time one would expect it to be the browser
type or version).
The code could certainly be altered to not allow the property name to be
specified at all, and instead only ever use navigator.userAgent. It does that
now when you dont specify a property name. Would that alleviate your concerns,
or do we need to switch to them specifying things as a browser name and version
pair instead of regular expressions and have the browser name and version
determined by svgweb's browser detection that runs at the beginning of
everything?
Original comment by bduncan%...@gtempaccount.com
on 26 Jul 2010 at 9:20
I think you're probably right that the way you are doing things doesn't allow
for XSS attacks.
The browser checking code is very mature and is taken from Dojo. I don't think
you will be happy doing userAgent and version checking as browser detection is
quite tricky and counter-intuitive. I think we can make what you are doing much
easier by being able to specify a browser, version, and perhaps a "less than",
"equal to", or "greater than" specifier. So being able to say:
Force flash for Internet Explorer less than 8
Use Flash for Firefox less than 1.5
etc.
without resorting to having to specify a regular expression. If you can come up
with a simple easy to use way of specifying this without using regular
expressions, I think it will be more robust and easier to use.
Original comment by bradneub...@gmail.com
on 26 Jul 2010 at 10:51
Original issue reported on code.google.com by
grick23@gmail.com
on 3 Sep 2009 at 7:12