Skip to content

Need to create a new transformation function called removeComments #408

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

Closed
rcbarnett-zz opened this issue Oct 17, 2013 · 5 comments
Closed
Assignees

Comments

@rcbarnett-zz
Copy link
Contributor

MODSEC-254: The current transformation function replaceComments is not sufficient for two cases -

  1. It is replaced with a space char which can cause false negatives when the SQL comments are inserted directly into SQL keywords like this -

?param=UNI/blah/ON+SEL/blah/ECT

The resulting payload would become -

UNI ON SEL ECT

And this would evade signatures that look for these keywords.

  1. If the payloads has unterminated SQL comments like this -

?param=1'UNION/!0SELECT user,2,3,4,5,6,7,8,9/!0from/!0mysql.user/-

The resulting payload would become -

1'UNION

We need a new tfns called removeComments that simply strips any /.../, or /* strings in place but does not remove any other text and does not replace it with spaces

@ghost ghost assigned zimmerle Oct 17, 2013
@rcbarnett-zz
Copy link
Contributor Author

Original reporter: rbarnett

@rcbarnett-zz
Copy link
Contributor Author

ivanr: Which databases allow comments to be inside keywords? It didn't work with the few databases I tried with, but it is rumoured that it works with MySQL 3.x.

@rcbarnett-zz
Copy link
Contributor Author

rbarnett: Good point Ivan. It seems that perhaps it worked at some point long ago in MySQL but doesn't any longer. I guess this erroneous bypass technique has stuck around in many SQL Injection cheatsheets/whitepapers and thus both tools and people attempt it. Even though this evasion technique might not actually work, the fact is that attackers/tools will still send it :) So, the question then becomes do you want your rules to still be able to generate alerts even then the attack has little chance of actually succeeding? Most users I have interacted with say yes.

Issue #2 is still a concern as MySQL still executed the the example SQLi payload. Perhaps it was was due to the /*! syntax and then using the -- comment at the end.

With all this in mind, perhaps we don't need a new operator but we should update the existing replaceComments tfns so that it strips out unterminated /* characters themselves but does not consider everything after it as a comment.

@rcbarnett-zz
Copy link
Contributor Author

rbarnett: I believe that I have identified the use case for attackers adding SQL comments inside SQL keywords - the scenario is when intermediate security devices attempt to cleanse the payloads. For instance, if a WAF/IPS tries to simply strip out SQL comments and then passes it onto the app/db, then the payload becomes the correct syntax.

@rcbarnett-zz
Copy link
Contributor Author

rbarnett: I think that we need to do two things -

  1. Update the way the current t:replaceComments code handles unterminated comments (/* with no closing /). In this case, it should just strip the / chars
  2. Add t:removeComments which simply strips out all comment chars - /, /!, */, --, #

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

2 participants