Skip to content

Commit

Permalink
Fix missing null checks in uses of consumeIdentOrUrlOrFunctions (#266)
Browse files Browse the repository at this point in the history
CssTokens code assumed that consumeIdentOrUrlOrFunctions always
returned a token type and consumed characters.

This commit audits all uses of that function and checks that
they make progress.
  • Loading branch information
mikesamuel authored Jun 8, 2022
1 parent 5372c74 commit c2c74fc
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
32 changes: 27 additions & 5 deletions src/main/java/org/owasp/html/CssTokens.java
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ void lex() {
char ch = css.charAt(pos);
int startOfToken = pos;
int startOfOutputToken = sb.length();
final TokenType type;
TokenType type;
switch (ch) {
case '\t': case '\n': case '\f': case '\r': case ' ': case '\ufeff':
consumeIgnorable();
Expand Down Expand Up @@ -514,6 +514,7 @@ void lex() {
type = TokenType.UNICODE_RANGE;
} else {
type = consumeIdentOrUrlOrFunction();
assert type != null;
}
break;
case '0': case '1': case '2': case '3': case '4':
Expand All @@ -533,7 +534,14 @@ && isDecimal(css.charAt(pos + 2)))) {
if (consumeIgnorable()) { // -->
type = TokenType.WHITESPACE;
} else {
type = consumeIdentOrUrlOrFunction();
TokenType identType = consumeIdentOrUrlOrFunction();
if (identType == null) {
breakOutput();
consumeDelim(ch);
type = TokenType.DELIM;
} else {
type = identType;
}
}
} else if (isIdentPart(lookahead)) {
// treat ".<IDENT>" as one token.
Expand Down Expand Up @@ -589,9 +597,17 @@ && isDecimal(css.charAt(pos + 2)))) {
}
break;
}
case '_':
type = consumeIdentOrUrlOrFunction();
case '_': {
TokenType identType = consumeIdentOrUrlOrFunction();
if (identType != null) {
type = identType;
} else {
++pos; // drop
breakOutput();
type = TokenType.WHITESPACE;
}
break;
}
case '\\': {
// Optimistically parse as an ident.
TokenType identType = consumeIdentOrUrlOrFunction();
Expand Down Expand Up @@ -624,7 +640,13 @@ && isDecimal(css.charAt(pos + 2)))) {
type = TokenType.WHITESPACE;
}
}
assert pos > startOfToken
// Make progress even in the face of errors above.
if (type == null && pos == startOfToken) {
type = TokenType.WHITESPACE;
breakOutput();
++pos;
}
assert type != null && pos > startOfToken
: "empty token at " + pos + ", ch0=" + css.charAt(startOfToken)
+ ":U+" + Integer.toHexString(css.charAt(startOfToken));
int endOfOutputToken = sb.length();
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/org/owasp/html/HtmlSanitizerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,13 @@ public static final void testIssue254SemicolonlessNamedCharactersInUrls() {
assertEquals(want, sanitize(input));
}

@Test
public static final void testStylingCornerCase() {
String input = "<a style=\\006-\\000038";
String want = "";
assertEquals(want, sanitize(input));
}

private static String sanitize(@Nullable String html) {
StringBuilder sb = new StringBuilder();
HtmlStreamRenderer renderer = HtmlStreamRenderer.create(
Expand Down

0 comments on commit c2c74fc

Please sign in to comment.