Skip to content

Commit

Permalink
Make HtmlEscape escape '/' again in UNKNOWN mode.
Browse files Browse the repository at this point in the history
This is a XSS-prevention recommendation.
If escaped code is only ever used inside a quoted attribute or as element text,
escapeing '/' is not necessary.
However, if the escaped code is inserted inside a tag (for example assuming
that it is a well-behavde attribute), then a slash may be meaningful in some
cases. Lots of other things can go wrong in that case, so we recommend against
it.

R=sgjesse@google.com

Review URL: https://codereview.chromium.org//1084473003

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@45153 260f80e4-7a28-3924-810f-c04153c831b5
  • Loading branch information
lrhn committed Apr 15, 2015
1 parent f79bc25 commit 8a5d049
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 13 deletions.
51 changes: 42 additions & 9 deletions sdk/lib/convert/html_escape.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,26 @@

part of dart.convert;

// TODO(floitsch) - Document - Issue 13097
/**
* A `String` converter that converts characters to HTML entities.
*
* This is intended to sanitice text before inserting the text into an HTML
* document. Characters that are meaningful in HTML are converted to
* HTML entities (like `&` for `&`).
*
* The general converter escapes all characters that are meaningful in HTML
* attributes or normal element context. Elements with special content types
* (like CSS or JavaScript) may need a more specialized escaping that
* understands that content type.
*
* If the context where the text will be inserted is known in more detail,
* it's possible to omit escaping some characters (like quotes when not
* inside an attribute value).
*
* The escaped text should only be used inside quoted HTML attributes values
* or as text content of a normal element. Using the escaped text inside a
* tag, but not inside a quoted attribute value, is still dangerous.
*/
const HtmlEscape HTML_ESCAPE = const HtmlEscape();

/**
Expand All @@ -28,6 +47,13 @@ class HtmlEscapeMode {
final bool escapeQuot;
/** Whether to escape "'" (apostrophe). */
final bool escapeApos;
/**
* Whether to escape "/" (forward slash, solidus).
*
* Escaping a slash is recommended to avoid cross-site scripting attacks by
* [the Open Web Application Security Project](https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content)
*/
final bool escapeSlash;

/**
* Default escaping mode which escape all characters.
Expand All @@ -40,7 +66,7 @@ class HtmlEscapeMode {
* which require escapes matching their particular content syntax.
*/
static const HtmlEscapeMode UNKNOWN =
const HtmlEscapeMode._('unknown', true, true, true);
const HtmlEscapeMode._('unknown', true, true, true, true);

/**
* Escaping mode for text going into double-quoted HTML attribute values.
Expand All @@ -51,7 +77,7 @@ class HtmlEscapeMode {
* Escapes only double quotes (`"`) but not single quotes (`'`).
*/
static const HtmlEscapeMode ATTRIBUTE =
const HtmlEscapeMode._('attribute', false, true, false);
const HtmlEscapeMode._('attribute', false, true, false, false);

/**
* Escaping mode for text going into single-quoted HTML attribute values.
Expand All @@ -62,7 +88,7 @@ class HtmlEscapeMode {
* Escapes only single quotes (`'`) but not double quotes (`"`).
*/
static const HtmlEscapeMode SQ_ATTRIBUTE =
const HtmlEscapeMode._('attribute', false, false, true);
const HtmlEscapeMode._('attribute', false, false, true, false);

/**
* Escaping mode for text going into HTML element content.
Expand All @@ -74,22 +100,26 @@ class HtmlEscapeMode {
* Escapes `<` and `>` characters.
*/
static const HtmlEscapeMode ELEMENT =
const HtmlEscapeMode._('element', true, false, false);
const HtmlEscapeMode._('element', true, false, false, false);

const HtmlEscapeMode._(
this._name, this.escapeLtGt, this.escapeQuot, this.escapeApos);
const HtmlEscapeMode._(this._name,
this.escapeLtGt,
this.escapeQuot,
this.escapeApos,
this.escapeSlash);

/**
* Create a custom escaping mode.
*
* All modes escape `&`.
* The mode can further be set to escape `<` and `>` ([escapeLtGt]),
* `"` ([escapeQuot]) and/or `'` ([escapeApos]).
* `"` ([escapeQuot]), `'` ([escapeApos]), and/or `/` ([escapeSlash]).
*/
const HtmlEscapeMode({String name: "custom",
this.escapeLtGt: false,
this.escapeQuot: false,
this.escapeApos: false}) : _name = name;
this.escapeApos: false,
this.escapeSlash: false}) : _name = name;

String toString() => _name;
}
Expand All @@ -108,6 +138,8 @@ class HtmlEscapeMode {
* * `'` (apostrophe) when inside a single-quoted attribute value.
* Apostrophe is escaped as `&#39;` instead of `&apos;` since
* not all browsers understand `&apos;`.
* * `/` (slash) is recommended to be escaped because it may be used
* to terminate an element in some HTML dialects.
*
* Escaping `>` (greater than) isn't necessary, but the result is often
* found to be easier to read if greater-than is also escaped whenever
Expand Down Expand Up @@ -150,6 +182,7 @@ class HtmlEscape extends Converter<String, String> {
case "'": if (mode.escapeApos) replacement = '&#39;'; break;
case '<': if (mode.escapeLtGt) replacement = '&lt;'; break;
case '>': if (mode.escapeLtGt) replacement = '&gt;'; break;
case '/': if (mode.escapeSlash) replacement = '&#46;'; break;
}
if (replacement != null) {
if (result == null) result = new StringBuffer();
Expand Down
7 changes: 3 additions & 4 deletions tests/lib/convert/html_escape_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const _NOOP = 'Nothing_to_escape';

const _TEST_INPUT = """<A </test> of \xA0 "double" & 'single' values>""";

const _OUTPUT_UNKNOWN = '&lt;A &lt;/test&gt; of \xA0 &quot;double&quot; &amp; '
'&#39;single&#39; values&gt;';
const _OUTPUT_UNKNOWN = '&lt;A &lt;&#46;test&gt; of \xA0 &quot;double&quot; '
'&amp; &#39;single&#39; values&gt;';

const _OUTPUT_ATTRIBUTE =
"<A </test> of \xA0 &quot;double&quot; &amp; 'single' values>";
Expand Down Expand Up @@ -48,8 +48,7 @@ void _testConvert(HtmlEscape escape, String input, String expected) {
void _testTransform(HtmlEscape escape, String input, String expected) {
var controller = new StreamController(sync: true);

var stream = controller.stream
.transform(escape);
var stream = controller.stream.transform(escape);

var done = false;
int count = 0;
Expand Down

0 comments on commit 8a5d049

Please sign in to comment.