enygma / expose

An Intrusion Detection System library loosely based on PHP IDS
MIT License
266 stars 55 forks source link

Expose doesn't detect encoded variables #31

Closed geralt closed 8 years ago

geralt commented 9 years ago

If you do a test with the example code from [1] and change the $data variable adding 'baz':

$data = array( 'POST' => array( 'test1' => 'foo', 'bar' => array( 'baz' => '%3C%69%6D%67%20%73%72%63%3D%22%22%20%6F%6E%65%72%72%6F%72%3D%22%6A%61%76%61%73%63%72%69%70%74%3A%64%6F%63%75%6D%65%6E%74%2E%77%72%69%74%65%22%3E', 'testing' => '<script>test</script>', 'path' => '../transversal/path' ) ), );

Expose doesn't detect attack on 'baz' variable ( baz's value = HTTP_encoded ('<img src="" onerror="javascript:document.write">') ). Any encoded variable isn't detected. If you see how PHPIDS do this you'll see that there is a class, IDS_Converter [2], to check these encoded variables.

[1] https://github.com/enygma/expose [2] https://github.com/PHPIDS/PHPIDS/blob/master/lib/IDS/Converter.php

geralt commented 9 years ago

Nothing to say about that?

enygma commented 9 years ago

honestly I just haven't gotten back around to responding. This project was something I was working on for a while to fill a need but I've moved on from that project. I could definitely see something like that being a nice addition, though. I'd hate to lift what it's done directly from that class, but it looks like they have it pretty well worked out for lots of different kinds of encodings.

enygma commented 9 years ago

Hmm, the "converter" idea is interesting - looks like PHPIDS has done a lot of work on that too. I don't want to just copy over the logic but I wonder if there's a better way. I'd like to have Expose be smart enough to detect encoded variables and their types and not just blindly have to apply converters. Not sure what @lstrojny might think on the subject...

lstrojny commented 8 years ago

The general issue here is, that you might receive payloads that target systems that rely on encodings you have a hard time figuring out. So the Converter is always going to be an arms-race (but that’s something that is true for an IDS in general). That being said, I would probably try a different approach nowadays and that is running detection on different permutations until you hit something (with a level based exit condition). That could even be computationally effective as you can abort once you hit a certainty >x. The conversion algorithm would look something like this:

for d in convert(v) -> [v1, v2, v3, …]
   detect(d)

That way you could even catch multiple encodings by trying different permutations of encodings. On an implementation side I would probably think about it as an iterator that goes through each value and permutation of encodings until it reached its end.

Awnage commented 8 years ago

Like you point out @lstrojny, the order that the conversion occurs is important. For giggles I ran this converter twice before sending to the filters and some vectors were rated even higher.

geralt commented 8 years ago

Here is my silly question: what is the diference between PHPID's Converter and your new code? PHPID's Converter executes it's methods following their position into the class code. Your code executes methods as function runAllConversions() says. Is it not the same?

Awnage commented 8 years ago

You are correct @geralt In fact, to get a jump start on the issue, I received permission from @lstrojny (from the PHPIDS project) to forklift and modify their converter code.