Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IllegalStateException on rgb percentage values #547

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+68 −3
Open

Fix IllegalStateException on rgb percentage values #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90581de
fix(gh-546): bug which resulted in an IllegalStateException for perce…
jonah1und1 Jan 20, 2025
6b73677
fix(gh-546): keep colour percentages in sanitized css instead of conv…
jonah1und1 Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/main/java/org/owasp/validator/css/CssValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*/
package org.owasp.validator.css;

import java.text.DecimalFormat;
import java.util.Iterator;
import java.util.regex.Pattern;
import org.owasp.validator.html.Policy;
Expand Down Expand Up @@ -344,15 +345,15 @@ public String lexicalValueToString(LexicalUnit lu) {
// this is a rgb encoded color
StringBuffer sb = new StringBuffer("rgb(");
LexicalUnit param = lu.getParameters();
sb.append(param.getIntegerValue()); // R value
sb.append(getColorValue(param)); // R value
sb.append(',');
param = param.getNextLexicalUnit(); // comma
param = param.getNextLexicalUnit(); // G value
sb.append(param.getIntegerValue());
sb.append(getColorValue(param));
sb.append(',');
param = param.getNextLexicalUnit(); // comma
param = param.getNextLexicalUnit(); // B value
sb.append(param.getIntegerValue());
sb.append(getColorValue(param));
sb.append(')');

return sb.toString();
Expand Down Expand Up @@ -404,4 +405,20 @@ public String lexicalValueToString(LexicalUnit lu) {
return null;
}
}

/**
* Returns color value as int.
* Maps percentages to values between 0 and 255.
* Negative percentages are mapped to 0, values bigger than 100% to 255.
*
* @param param LexicalUnit
* @return color value as int
*/
private static String getColorValue(LexicalUnit param) {
if (param.getLexicalUnitType() == LexicalUnit.SAC_PERCENTAGE) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonah1und1 knowing we are dealing with building a string and it is possible to detect if we are handling a percentage, don't you think it would be most suitable and reliable in terms of matching the original input to reconstruct the percentage as a string? I know this affects your proposed method signature returning an int, but we always try to be as much equal as possible to the input if we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I refactored the method.

return new DecimalFormat("0.#").format(param.getFloatValue()) + "%";
} else {
return "" + param.getIntegerValue();
}
}
}
18 changes: 18 additions & 0 deletions src/main/resources/antisamy.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@
<regexp name="rgbCode"
value="rgb\(([1]?[0-9]{1,2}|2[0-4][0-9]|25[0-5]),([1]?[0-9]{1,2}|2[0-4][0-9]|25[0-5]),([1]?[0-9]{1,2}|2[0-4][0-9]|25[0-5])\)" />

<!-- Relative color values between 0%-100% are also allowed -->
<regexp name="rgbPercentage"
value="rgb\(([0-9][0-9]?|100)\.?\d*%,([0-9][0-9]?|100)\.?\d*%,([0-9][0-9]?|100)\.?\d*%\)" />

<!-- CSS2 Allowed System Color Values -->
<regexp name="systemColor"
value="(activeborder|activecaption|appworkspace|background|buttonface|buttonhighlight|buttonshadow|buttontext|captiontext|graytext|highlight|highlighttext|inactiveborder|inactivecaption|inactivecaptiontext|infobackground|infotext|menu|menutext|scrollbar|threeddarkshadow|threedface|threedhighlight|threedlightshadow|threedshadow|window|windowframe|windowtext)" />
Expand Down Expand Up @@ -1141,6 +1145,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand Down Expand Up @@ -1207,6 +1212,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand All @@ -1221,6 +1227,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand All @@ -1235,6 +1242,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand All @@ -1249,6 +1257,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand All @@ -1263,6 +1272,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand Down Expand Up @@ -1314,6 +1324,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand Down Expand Up @@ -1693,6 +1704,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
</property>
Expand Down Expand Up @@ -2348,6 +2360,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
<shorthand-list>
Expand All @@ -2366,6 +2379,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
<shorthand-list>
Expand All @@ -2384,6 +2398,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
<shorthand-list>
Expand All @@ -2402,6 +2417,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
<shorthand-list>
Expand All @@ -2420,6 +2436,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
</regexp-list>
<shorthand-list>
Expand Down Expand Up @@ -2587,6 +2604,7 @@
<regexp name="colorName" />
<regexp name="colorCode" />
<regexp name="rgbCode" />
<regexp name="rgbPercentage" />
<regexp name="systemColor" />
<regexp name="length" />
</regexp-list>
Expand Down
30 changes: 30 additions & 0 deletions src/test/java/org/owasp/validator/html/test/AntiSamyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2725,4 +2725,34 @@ public void testGithubIssue484() throws ScanException, PolicyException {
assertEquals("<p>this is para data</p>\n" + "<br/>\n" + "<p>this is para data 2</p>", domValue);
assertEquals("<p>this is para data</p>\n" + "<br/>\n" + "<p>this is para data 2</p>", saxValue);
}

@Test
public void testGithubIssue546() throws ScanException, PolicyException {
//Given
String taintedHtml = "<style>.cl { color: rgb(50%, 20.5%, 100%); }</style>";

//When
CleanResults crDom = as.scan(taintedHtml, policy, AntiSamy.DOM);
CleanResults crSax = as.scan(taintedHtml, policy, AntiSamy.SAX);

//Then
String expectedCleanHtml = "<style>*.cl {\n\tcolor: rgb(50%,20.5%,100%);\n}\n</style>";
assertEquals(expectedCleanHtml, crDom.getCleanHTML());
assertEquals(expectedCleanHtml, crSax.getCleanHTML());
}

@Test
public void testGithubIssue546FaultyPercentagesGetFilteredByRegex() throws ScanException, PolicyException {
//Given
String taintedHtml = "<style>.cl { color: rgb(50%, -20%, 150%); }</style>";

//When
CleanResults crDom = as.scan(taintedHtml, policy, AntiSamy.DOM);
CleanResults crSax = as.scan(taintedHtml, policy, AntiSamy.SAX);

//Then
String expectedCleanHtml = "<style>*.cl {\n}\n</style>";
assertEquals(expectedCleanHtml, crDom.getCleanHTML());
assertEquals(expectedCleanHtml, crSax.getCleanHTML());
}
}
Loading