Skip to content

Commit 31ee6eb

Browse files
committed
Address github issue #62.
1 parent 8120aba commit 31ee6eb

File tree

3 files changed

+41
-7
lines changed

3 files changed

+41
-7
lines changed

pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<groupId>org.owasp.antisamy</groupId>
44
<artifactId>antisamy</artifactId>
55
<packaging>jar</packaging>
6-
<version>1.5.12</version>
6+
<version>1.5.13</version>
77

88
<distributionManagement>
99
<snapshotRepository>

src/main/java/org/owasp/validator/html/scan/AntiSamyDOMScanner.java

+13-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2007-2020, Arshan Dabirsiaghi, Jason Li
2+
* Copyright (c) 2007-2021, Arshan Dabirsiaghi, Jason Li
33
*
44
* All rights reserved.
55
*
@@ -38,7 +38,16 @@
3838
import org.owasp.validator.html.model.Tag;
3939
import org.owasp.validator.html.util.ErrorMessageUtil;
4040
import org.owasp.validator.html.util.HTMLEntityEncoder;
41-
import org.w3c.dom.*;
41+
import org.w3c.dom.Comment;
42+
import org.w3c.dom.DOMException;
43+
import org.w3c.dom.Document;
44+
import org.w3c.dom.DocumentFragment;
45+
import org.w3c.dom.Element;
46+
import org.w3c.dom.NamedNodeMap;
47+
import org.w3c.dom.Node;
48+
import org.w3c.dom.NodeList;
49+
import org.w3c.dom.ProcessingInstruction;
50+
import org.w3c.dom.Text;
4251
import org.xml.sax.InputSource;
4352
import org.xml.sax.SAXException;
4453
import org.xml.sax.SAXNotRecognizedException;
@@ -47,7 +56,8 @@
4756
import java.io.IOException;
4857
import java.io.StringReader;
4958
import java.io.StringWriter;
50-
import java.util.*;
59+
import java.util.List;
60+
import java.util.Queue;
5161
import java.util.concurrent.Callable;
5262
import java.util.concurrent.ConcurrentLinkedQueue;
5363
import java.util.regex.Matcher;
@@ -620,7 +630,6 @@ private void processChildren(NodeList childNodes, int currentStackDepth ) throws
620630
private void removePI(Node node) {
621631
addError(ErrorMessageUtil.ERROR_PI_FOUND, new Object[]{HTMLEntityEncoder.htmlEntityEncode(node.getTextContent())});
622632
removeNode(node);
623-
node.getParentNode().removeChild(node);
624633
}
625634

626635
private void stripCData(Node node) {

src/test/java/org/owasp/validator/html/test/AntiSamyTest.java

+27-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2007-2020, Arshan Dabirsiaghi, Jason Li
2+
* Copyright (c) 2007-2021, Arshan Dabirsiaghi, Jason Li
33
*
44
* All rights reserved.
55
*
@@ -35,6 +35,7 @@
3535
import static org.junit.Assert.fail;
3636
import static org.hamcrest.CoreMatchers.containsString;
3737
import static org.hamcrest.CoreMatchers.equalTo;
38+
import static org.hamcrest.CoreMatchers.is;
3839
import static org.hamcrest.CoreMatchers.not;
3940
import static org.hamcrest.MatcherAssert.assertThat;
4041

@@ -62,7 +63,6 @@
6263
import org.owasp.validator.html.model.Attribute;
6364
import org.owasp.validator.html.model.Tag;
6465

65-
6666
/**
6767
* This class tests AntiSamy functionality and the basic policy file which
6868
* should be immune to XSS and CSS phishing attacks.
@@ -1454,4 +1454,29 @@ public void testGithubIssue48() throws ScanException, PolicyException {
14541454
assertThat(as.scan(danglingMarkup2, policy, AntiSamy.SAX).getCleanHTML(), not(containsString("//evilactor.com/")));
14551455
assertThat(as.scan(danglingMarkup2, policy, AntiSamy.DOM).getCleanHTML(), not(containsString("//evilactor.com/")));
14561456
}
1457+
1458+
@Test
1459+
public void testGithubIssue62() {
1460+
// Concern is that when a processing instruction is at the root level, node removal gets messy and Null pointer exception arises.
1461+
// More test cases are added for PI removal.
1462+
1463+
try{
1464+
assertThat(as.scan("|<?ai aaa", policy, AntiSamy.DOM).getCleanHTML(), is("|"));
1465+
assertThat(as.scan("|<?ai aaa", policy, AntiSamy.SAX).getCleanHTML(), is("|"));
1466+
1467+
assertThat(as.scan("<div>|<?ai aaa", policy, AntiSamy.DOM).getCleanHTML(), is("<div>|</div>"));
1468+
assertThat(as.scan("<div>|<?ai aaa", policy, AntiSamy.SAX).getCleanHTML(), is("<div>|</div>"));
1469+
1470+
assertThat(as.scan("<div><?foo note=\"I am XML processing instruction. I wish to be excluded\" ?></div>", policy, AntiSamy.DOM)
1471+
.getCleanHTML(), not(containsString("<?foo")));
1472+
assertThat(as.scan("<div><?foo note=\"I am XML processing instruction. I wish to be excluded\" ?></div>", policy, AntiSamy.SAX)
1473+
.getCleanHTML(), not(containsString("<?foo")));
1474+
1475+
assertThat(as.scan("<?xml-stylesheet type=\"text/css\" href=\"style.css\"?>", policy, AntiSamy.DOM).getCleanHTML(), is(""));
1476+
assertThat(as.scan("<?xml-stylesheet type=\"text/css\" href=\"style.css\"?>", policy, AntiSamy.SAX).getCleanHTML(), is(""));
1477+
1478+
} catch (Exception exc) {
1479+
fail(exc.getMessage());
1480+
}
1481+
}
14571482
}

0 commit comments

Comments
 (0)