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.

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

NullPointerException for input string "|<?ai aaa" #62

Closed
eddierock1994 opened this issue Jan 11, 2021 · 4 comments
Closed

NullPointerException for input string "|<?ai aaa" #62

eddierock1994 opened this issue Jan 11, 2021 · 4 comments

Comments

@eddierock1994
Copy link

The code in question gets invoked through ESAPI library so I'm not sure if it has a bearing. But on examination of the method removePI of class AntiSamyDOMScanner, it looks like the node doesn't have any parent node. And so node.getParentNode() creates a null pointer exception. Attaching the stack trace here.

java.lang.NullPointerException
at org.owasp.validator.html.scan.AntiSamyDOMScanner.removePI(AntiSamyDOMScanner.java:689)
at org.owasp.validator.html.scan.AntiSamyDOMScanner.recursiveValidateTag(AntiSamyDOMScanner.java:260)
at org.owasp.validator.html.scan.AntiSamyDOMScanner.processChildren(AntiSamyDOMScanner.java:675)
at org.owasp.validator.html.scan.AntiSamyDOMScanner.processChildren(AntiSamyDOMScanner.java:666)
at org.owasp.validator.html.scan.AntiSamyDOMScanner.scan(AntiSamyDOMScanner.java:159)
at org.owasp.validator.html.AntiSamy.scan(AntiSamy.java:93)

@spassarop
Copy link
Collaborator

spassarop commented Jan 11, 2021

Hi @eddierock1994, thanks for the analysis. You're right, the problem in removePI is node.getParentNode().removeChild(node);.

In fact, after analyzing the issue I realized that the line is not even necessary. That's because the previous removeNode(node); call already does the same exact task, so the last line became redundant and conflictive. I think the best solution is to remove it.

I've added a unit test with four cases, soon to be uploaded in a PR with the suggested change. To make it easier for everyone to understand the problem (luckily this was not the case) and have a starting point in future reported issues, provide a JUnit test :)

@spassarop spassarop mentioned this issue Jan 12, 2021
@eddierock1994
Copy link
Author

Hi @spassarop will provide a Junit test in the future. Sorry it's my first time logging an issue.

davewichers added a commit that referenced this issue Jan 13, 2021
@davewichers
Copy link
Collaborator

This has been fixed in release 1.5.13

@eddierock1994
Copy link
Author

Thanks for the quick resolution. You guys are great :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants