1

I have the following code in my frontend javascript which basically reads the csrf cookie value and sets that in the ajax calls done via jquery.

    var csrftoken = self.getCookie('csrftoken');
    xhr.setRequestHeader("X-CSRFToken", csrftoken);

This seems to be a very standard technique and yet Veracode reports it as a vulnerability.

Looking at the details of this kind of vulnerability, at https://cwe.mitre.org/data/definitions/113.html, I don't see how could it be an issue given the http header is being set from client end and not server end. If the csrf token value is injected wrongly, the request would not suceed due to csrf mismatch anyways.

Why does Veracode consider this to be a vulnerability at all or is it a vulnerability that I am unable to understand?

Divick
  • 121
  • 2

2 Answers2

0

HTTP response headers are delimited by one line break, and two line breaks start the actual response body. If an attacker can inject a line break into the header they can inject a new header after the line break, and if they inject two they can also inject content into the response body, such as new HTML (XSS). This is might happen because of Improper Neutralization of CRLF Sequences in HTTP Headers (AKA "HTTP Response Splitting").

These days, this issue is very unlikely – practically nobody uses a single addHeader() sink to add multiple headers or addCookie() to add multiple cookies - you just invoke it multiple times if the need arises. All modern frameworks handle that and make sure that the splitting, caused by injection, is mitigated.

If somebody still insists implementing an HTTP writer over TCP – they could always introduce header-splitting as they write the raw packet, but that is an unlikely scenario, and it will not be caught by Static Analysis Tools like Veracode, because they would never be able to get the context of the packet.

The bottom line is that this Veracode result seems like a False Positive.

yaloner
  • 250
  • 1
  • 6
  • 1
    From what I can make out from your response is that addHeader and addCookie would be an issue on the server side, if the framework does not handle that correctly and allows adding multiple headers and/or cookies, but in this case the code in question is on the client side. So the question really boils down to from the perspective of client side code, is it even possible? Veracode seems to be just checking for code which matches a pattern and does not care about where the code is being run and hence the red flag. BTW I don't couldn't really understand the latter part of your answer. – Divick Jan 07 '20 at 10:10
0

From what I can tell, it's a false-positive.

Generally, CRLF injection is a server-side issue, though theoretically it could also work client-side.

For this to be vulnerable, you'd need:

  • setRequestHeader to be vulnerable to CRLF injection. Current versions of Chrome and Firefox are not (which is to be expected; such a behavior would be a vulnerability in the browser; the most recent case of such a vulnerability I could find is from 2007 in Safari).
  • the input to be attacker-controlled. In your case, if an attacker can set a CSRF token for another user, you'd already have a pretty severe problem.
  • a way to exploit it. The advisory for the Safari issue linked above has an example for this: By injecting a new Host header, an attacker could get the victim to send a request to an attacker-controlled server (instead of the application-controlled server). If the response is handled insecurely, this could eg lead to XSS. This may also disclose a users CSRF token to the attacker-controlled server. Another possible attack vector would be to inject two newlines, followed by an attacker-controlled HTTP body (to perform a direct CSRF attack if the CSRF header is above the injection point).

While the attack scenarios would be really interesting, it is a theoretical issue given that setRequestHeader shouldn't be vulnerable to CRLF. As-is, I don't see a need to change the code.

tim
  • 29,018
  • 7
  • 95
  • 119