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

AntiSamy is not work on <svg/onload = alert('Hello')/> #47

Closed
niketsclarion opened this issue Jul 10, 2020 · 5 comments
Closed

AntiSamy is not work on <svg/onload = alert('Hello')/> #47

niketsclarion opened this issue Jul 10, 2020 · 5 comments

Comments

@niketsclarion
Copy link

I am working on 1 of XSS issue where our tester finds an issue like <svg/onload = alert('Hello') > and antisamy is not cleaning this particular tag.

even I debug antisamy library that it will consider or <style> as a tag and continue with current code so it is not throwing any particular exception.

i have already written small test case for your reference

@Test public void testStyleOnloadWithAlertScripts() throws PolicyException, ScanException { assertEquals( "", scanner.scan("<style/onload = alert(document.domain)>")); }

can anyone look into it to resolving this issue either from XML Configuration or from new patch release

@davewichers
Copy link
Collaborator

I just added the following test case to AntiSamyTest.java and it passes no problem. It includes a test that matches your issue title (ending with />) as well as a test without the / at the end, and then the <style/onload version of the test you described in the 'test case for reference' you provided, which actually looks more like issue #44.

@Test
public void testGithubIssue47() throws PolicyException, ScanException {
	assertEquals( "", as.scan("<svg/onload = alert('Hello')>", policy, AntiSamy.SAX).getCleanHTML());
	assertEquals( "", as.scan("<svg/onload = alert('Hello')/>", policy, AntiSamy.DOM).getCleanHTML());
	assertEquals( "", as.scan("<style/onload = alert(document.domain)>", policy, AntiSamy.SAX).getCleanHTML());
	assertEquals( "", as.scan("<style/onload = alert(document.domain)>", policy, AntiSamy.DOM).getCleanHTML());
}

As such, I can't replicate the issue. Can you provide a pull request to AntiSamyTest.java with a failing test case that demonstrates your issue?

@niketsclarion
Copy link
Author

Hey Thanks,
But I am not getting any Scan Security Error CleanResults.getErrorMessages()
as my code is depending on an error we will modify it and that return text.

@niketsclarion
Copy link
Author

@davewichers Do You have any update for it.

@nahsra
Copy link
Owner

nahsra commented Aug 13, 2020

The getErrorMessages() API does not answer the question "is this safe input?" You must always use the sanitized input. The serialization and deserialization process is purposefully lossy and will filter attacks, but one of the tradeoffs is that we don't have an ability to see them in retrospect.

The JavaDoc and README.md should be more clear about that, so I think that should be the scope of the ticket.

@davewichers
Copy link
Collaborator

davewichers commented Aug 17, 2020

The README.md has been updated to be more clear on this, as has the JavaDoc, and these changes have been checked into 'master' in commit 75f9bb4. The JavaDoc changes will also be pushed out in the 1.5.11 release, which we are working on now but in the meantime, the README displayed on github for this project has the latest guidance on how getErrorMessages() works.

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