Skip to content

Commit 3d7925e

Browse files
committed
testcases updated, fast fail added
1 parent 5215a50 commit 3d7925e

File tree

3 files changed

+37
-7
lines changed

3 files changed

+37
-7
lines changed

src/main/java/com/github/bgalek/security/svg/SvgSecurityValidator.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66

77
import java.nio.charset.StandardCharsets;
88
import java.util.Arrays;
9+
import java.util.Collections;
910
import java.util.HashSet;
1011
import java.util.Objects;
1112
import java.util.Set;
13+
import java.util.regex.Pattern;
1214

1315
/**
1416
* SVG Safe is a very simple and lightweight library that helps
@@ -20,6 +22,8 @@
2022
*/
2123
public class SvgSecurityValidator implements XssDetector {
2224

25+
private static final Pattern JAVASCRIPT_PROTOCOL_IN_CSS_URL = Pattern.compile("url\\(.?javascript");
26+
2327
/**
2428
* This is the main method that handles svg file validation
2529
*
@@ -40,6 +44,7 @@ public ValidationResult validate(byte[] input) {
4044
}
4145

4246
private static Set<String> getOffendingElements(String xml) {
47+
if (JAVASCRIPT_PROTOCOL_IN_CSS_URL.matcher(xml).find()) return Collections.singleton("style");
4348
PolicyFactory policy = new HtmlPolicyBuilder()
4449
.allowElements(SVG_ELEMENTS)
4550
.allowStyling()
@@ -303,7 +308,10 @@ private static Set<String> getOffendingElements(String xml) {
303308
"x",
304309
"x1",
305310
"x2",
311+
"xlink:href",
306312
"xmlns",
313+
"xml:space",
314+
"xmlns:xlink",
307315
"y",
308316
"y1",
309317
"y2",

src/test/java/com/github/bgalek/security/svg/SvgSecurityValidatorTest.java

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package com.github.bgalek.security.svg;
22

33
import org.junit.jupiter.params.ParameterizedTest;
4+
import org.junit.jupiter.params.provider.Arguments;
5+
import org.junit.jupiter.params.provider.MethodSource;
46
import org.junit.jupiter.params.provider.ValueSource;
57

68
import java.io.File;
79
import java.io.IOException;
810
import java.nio.file.Files;
11+
import java.util.Collections;
912
import java.util.Objects;
13+
import java.util.stream.Stream;
1014

1115
import static org.junit.jupiter.api.Assertions.assertEquals;
1216
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -15,43 +19,53 @@
1519
class SvgSecurityValidatorTest {
1620

1721
@ParameterizedTest(name = "validate {0} svg")
18-
@ValueSource(strings = {"hacked/with-onclick-attribute.svg", "hacked/with-script-tag.svg", "hacked/with-script-tag-in-styles.svg"})
19-
void shouldDetectXssInFiles(String file) {
22+
@MethodSource("evilUseCases")
23+
void shouldDetectXssInFiles(String file, String expectedOffendingElements) {
2024
ValidationResult detect = new SvgSecurityValidator().validate(loadFile(file));
21-
assertEquals(1, detect.getOffendingElements().size());
2225
assertTrue(detect.hasViolations());
26+
assertEquals(expectedOffendingElements, String.join(",", detect.getOffendingElements()));
2327
}
2428

2529
@ParameterizedTest(name = "validate {0} svg")
2630
@ValueSource(strings = {"original/valid.svg"})
2731
void shouldNotDetectAnythingInValidFiles(String file) {
2832
ValidationResult detect = new SvgSecurityValidator().validate(loadFile(file));
29-
assertEquals(0, detect.getOffendingElements().size());
3033
assertFalse(detect.hasViolations());
34+
assertEquals(Collections.emptySet(), detect.getOffendingElements());
3135
}
3236

3337
@ParameterizedTest(name = "validate {0} svg")
3438
@ValueSource(strings = {"original/valid.svg"})
3539
void shouldNotDetectAnythingInValidFilesUsingBytes(String file) {
3640
ValidationResult detect = new SvgSecurityValidator().validate(loadFile(file).getBytes());
37-
assertEquals(0, detect.getOffendingElements().size());
3841
assertFalse(detect.hasViolations());
42+
assertEquals(Collections.emptySet(), detect.getOffendingElements());
3943
}
4044

4145
@ParameterizedTest(name = "validate {0} svg")
4246
@ValueSource(strings = {"broken/broken.csv.svg"})
4347
void shouldThrowExceptionWhenInputIsNotValidXml(String file) {
4448
ValidationResult detect = new SvgSecurityValidator().validate(loadFile(file));
45-
assertEquals(0, detect.getOffendingElements().size());
4649
assertFalse(detect.hasViolations());
50+
assertEquals(Collections.emptySet(), detect.getOffendingElements());
4751
}
4852

4953
@ParameterizedTest(name = "validate {0} svg")
5054
@ValueSource(strings = {"broken/broken.png.svg"})
5155
void shouldThrowExceptionWhenInputIsBinaryType(String file) {
5256
ValidationResult detect = new SvgSecurityValidator().validate(loadFile(file).getBytes());
53-
assertEquals(0, detect.getOffendingElements().size());
5457
assertFalse(detect.hasViolations());
58+
assertEquals(Collections.emptySet(), detect.getOffendingElements());
59+
}
60+
61+
private static Stream<Arguments> evilUseCases() {
62+
return Stream.of(
63+
Arguments.of("hacked/with-onclick-attribute.svg", "onclick"),
64+
Arguments.of("hacked/with-script-tag.svg", "script"),
65+
Arguments.of("hacked/with-script-tag-in-styles.svg", "script"),
66+
Arguments.of("hacked/with-css-url-syntax.svg", "style"),
67+
Arguments.of("hacked/with-xlink-injection.svg", "script")
68+
);
5569
}
5670

5771
private String loadFile(String fileName) {
Lines changed: 8 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)