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

Sanitized output for same tainted input differs from AntiSamy 1.7.3 to 1.7.4 #389

Closed
kwwall opened this issue Nov 11, 2023 · 6 comments
Closed

Comments

@kwwall
Copy link
Contributor

kwwall commented Nov 11, 2023

The release notes do not indicate that the sanitized output may different for AntiSamy between release 1.7.3 and 1.7.4, but here are 3 examples taken from ESAPI that show this is the case. Note that I have only tried this using the AntiSamy.DOM parser (which is what ESAPI uses). YMMV with AntiSamy.SAX parser.

See PR #388 for details.

My expectations here is that you update your 1.7.4 release notes and possibly mention this in your README.md file as it can potentially break people's regression tests against AntiSamy.

@kwwall
Copy link
Contributor Author

kwwall commented Nov 11, 2023

Incidentally, using SAX parser gives the same failed test results, but TBH, I'm not sure that is a fair test because I've never ran this test using the AntiSamy.SAX parser before so I don't know what the previous results with it were.

@spassarop
Copy link
Collaborator

I reviewed this and it looks like the HTML parser now handles differently when certain tags end or begin at the start but don't have a corresponding closing or opening tag.

Fortunately, it didn't end in a security bug so far. I guess the best would be what you suggest and add that extending note. As our checks may be more flexible to changes (we tend to look for bad stuff not appearing rather than comparing exact outputs) this did not arise on our tests. Thanks Kevin.

@davewichers
Copy link
Collaborator

@spassarop - Can you update something to address this? The README or whatever?

@spassarop
Copy link
Collaborator

spassarop commented Jan 12, 2024 via email

@davewichers
Copy link
Collaborator

OK. Closing this as this update will go out in release 1.7.5

@kwwall
Copy link
Contributor Author

kwwall commented Jan 21, 2024

Finally getting caught up on this. This was mostly an FYI anyhow and I was only mentioning it in case anyone had similar regression tests that are now failing, I just figured it we ought to let them know, so a comment in the README will address that. It still sanitizes safely and that's all that really should matter. Hence I agree this should be closed.

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