Adding overly permissive CORS policy detector #248
Conversation
|
Good idea ! |
|
|
||
| LDC ldc = ByteCode.getPrevInstruction(location.getHandle().getPrev(), LDC.class); | ||
| if (ldc != null) { | ||
| if ("Access-Control-Allow-Origin".equals(ldc.getValue(cpg)) && |
There was a problem hiding this comment.
I would also test for the same header with different case (access-control-allow-origin).
http://stackoverflow.com/a/5259004/89769
| LDC ldc = ByteCode.getPrevInstruction(location.getHandle().getPrev(), LDC.class); | ||
| if (ldc != null) { | ||
| if ("Access-Control-Allow-Origin".equals(ldc.getValue(cpg)) && | ||
| "*".equals(ByteCode.getConstantLDC(location.getHandle().getPrev(), cpg, String.class))) { |
There was a problem hiding this comment.
I would also test for wildcard use as sub-domain. (replace equals by contains)
http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
Also the exact value "null" could be detected
| String className = invoke.getClassName(cpg); | ||
|
|
||
| if (className.equals("javax.servlet.http.HttpServletResponse") && | ||
| (methodName.equals("addHeader") || methodName.equals("setHeader"))) { |
There was a problem hiding this comment.
Bonus : Typically website that need to use Access-Control-Allow-Origin will analyze the Origin header.
http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
This is not mandatory for this PR. The description will have to be guideline of how to analyse properly Origin header. I would be interested to write this one.
There was a problem hiding this comment.
Nice! Go ahead, I'm looking forward to seeing how you approach it.
|
Maybe it would make sense to move this detector in a Not sure I'd have time to do it, but similar checks specific to Spring Framework could be added (see https://docs.spring.io/spring-security/site/docs/current/reference/html/cors.html and http://docs.spring.io/spring/docs/current/spring-framework-reference/html/cors.html). |
| "*".equals(ByteCode.getConstantLDC(location.getHandle().getPrev(), cpg, String.class))) { | ||
| String headerValue = ByteCode.getConstantLDC(location.getHandle().getPrev(), cpg, String.class); | ||
| if ("Access-Control-Allow-Origin".equalsIgnoreCase((String)ldc.getValue(cpg)) && | ||
| (headerValue.contains("*") || "null".equalsIgnoreCase(headerValue))) { | ||
|
|
There was a problem hiding this comment.
Fixed case sensitivity, header value wildcards and "null" value.
Detector reports overly permissive CORS policy that sets asterisk as the value of the Access-Control-Allow-Origin header, like in the following examples:
response.addHeader("Access-Control-Allow-Origin", "*");
response.setHeader("Access-Control-Allow-Origin", "*");