googlearchive / TemplateBinding

TemplateBinding Prolyfill
290 stars 55 forks source link

protecting attributes from XSS? #58

Open jmesserly opened 11 years ago

jmesserly commented 11 years ago

MDV is pretty safe right now from innerHTML XSS bugs, because it uses textContent to bind text (see #57 for a request to loosen this up sometimes). However, attributes are unprotected. If the application developer allows users (aka attackers ;) ) to control an attribute, they can do bad things.

here is a demo:

<head>
  <script src="mdv.js"></script>
</head>
<body>
  <h1>XSS demo</h1>
  <template bind>
    <p>Type something like: <pre>javascript:alert('hello from XSS');</pre></p>
    <p>Then click the link!</p>
    <input value="{{attackerInput}}">
    <a href="{{attackerInput}}">Link to: {{attackerInput}}</a>
  </template>
<script>
document.querySelector('template').model = {
  attackerInput: ''
};
</script>
</body>

It seems like a good idea to sanitize URL attributes, similar to other template engines.

From the html5 spec index we got the following list:

var urlAttributes = [
  'action',     // in form
  'cite',       // in blockquote, del, ins, q
  'data',       // in object
  'formaction', // in button, input
  'href',       // in a, area, link, base, command
  'manifest',   // in html
  'poster',     // in video
  'src',        // in audio, embed, iframe, img, input, script, source, track,
                //    video
];

of course, you can do more sophisticated sanitization (see GWT UriUtils), but the following simple algorithm worked for us in conjunction with a SafeUri type: https://github.com/dart-lang/web-ui/blob/master/lib/templating.dart#L195

@sigmundch

arv commented 11 years ago

If we go down this route we also need to handle all on* attributes as well as URLs in style attributes and style elements.

Then there is srcdoc of iframes and probably a few more things.

If we want to do this we should should really talk to Mike Samuel. On Apr 24, 2013 2:27 PM, "John Messerly" notifications@github.com wrote:

MDV is pretty safe right now from innerHTML XSS bugs, because it uses textContent to bind text (see #57https://github.com/toolkitchen/mdv/issues/57for a request to loosen this up sometimes). However, attributes are unprotected. If the application developer allows users (aka attackers ;) ) to control an attribute, they can do bad things.

here is a demo:

XSS demo

It seems like a good idea to sanitize URL attributes, similar to other template engines. From the html5 spec indexhttp://www.whatwg.org/specs/web-apps/current-work/multipage/section-index.html#attributes-1we got the following list: var urlAttributes = [ 'action', // in form 'cite', // in blockquote, del, ins, q 'data', // in object 'formaction', // in button, input 'href', // in a, area, link, base, command 'manifest', // in html 'poster', // in video 'src', // in audio, embed, iframe, img, input, script, source, track, // video]; of course, you can do more sophisticated sanitization (see GWT UriUtilshttp://code.google.com/p/google-web-toolkit/source/browse/trunk/user/src/com/google/gwt/safehtml/shared/UriUtils.java), but the following simple algorithm worked for us in conjunction with a SafeUri type: https://github.com/dart-lang/web-ui/blob/master/lib/templating.dart#L195 @sigmundch https://github.com/sigmundch — Reply to this email directly or view it on GitHubhttps://github.com/toolkitchen/mdv/issues/58 .
jmesserly commented 11 years ago

agreed, would be good to discuss. @xtofian was interested in those discussions too