ASVS icon indicating copy to clipboard operation
ASVS copied to clipboard

proposal/new requirement - served filename in content-disposition header must follow correct encoding

Open elarlang opened this issue 2 years ago • 5 comments

Basically, it must follow RFC 6266: "Use of the Content-Disposition Header Field in the Hypertext Transfer Protocol (HTTP)" https://tools.ietf.org/html/rfc6266#section-5

Proposal (2023-04-29 updated requirement text and added alternative category):

  • Category: V12.5 "File and Resources > File Download" OR "V5.3 Output Encoding and Injection Prevention"
  • Verify that when Content-Disposition header is used then filename and filename* attribute values are correctly sanitized and encoded.
  • Level: 1, 2, 3
  • CWE: ?
    • CWE-172 - CWE-172: Encoding Error

Note: Content-Disposition header may be used with attachment or inline, so we can not limit requirement text only for "download file" functionality.

Why: if not converted correctly, it may give "header injection" possibility

Something like that (even this one is for mails):

  • https://github.com/PHPMailer/PHPMailer/security/advisories/GHSA-f7hx-fqxw-rvvj

Update:

  • filename - only characters from ISO-8859-1 can be used, value bust be sanitized
  • filename* - can be presented in chosen charset (utf-8) and need to be: sanitized + encoded to charset + urlencoded

elarlang avatar Sep 30 '22 16:09 elarlang

I'm commenting because I have been researching this issue.


Basically, it is better to conform to the RFC or WHAT WG. However, the WHATWG recommendation is URL Encode to filename.

WHATWG HTML Spec: https://html.spec.whatwg.org/#multipart-form-data

For field names and filenames for file fields, the result of the encoding in the previous bullet point must be escaped by replacing any 0x0A (LF) bytes with the byte sequence %0A, 0x0D (CR) with %0D and 0x22 (") with %22. The user agent must not perform any other escapes.

Chrome and Firefox use the URL Encode method.


HTTP Clinet and other systems basically used escaping with \ . The same is true of Email client. (e.g. Gmail)

  • " --> \"
  • \n --> \\n
  • \r --> \\r

" escaping is necessary because field names, file extension, etc.. are tampered with when filenames such as the following are inserted into Content-Disposition.

Input filename:
malicious.sh"; name="field2"; dummy="abc.txt


Output Content-Disposition:

Content-Disposition: form-data; name="field1"; filename="malicious.sh"; name="field2"; dummy="abc.txt"

Similarly, I have confirmed that filename* can be RFD if it can generate the following Content-Disposition when URL Encoding is not done. (This is only valid in Firefox, though.)

Content-Disposition: attachment; filename*=utf-8''malicious.sh%00'normal.txt

Examples of problematic cases include:

  • HTTP Request > multipart (HTTP Client & Browser)
    • https://github.com/jnunemaker/httparty/security/advisories/GHSA-5pq7-52mg-hr42
    • https://bugzilla.mozilla.org/show_bug.cgi?id=1556711
  • HTTP Response > HTTP Header (Web Framework)
    • https://github.com/advisories/GHSA-8x94-hmjh-97hq
    • https://security.snyk.io/vuln/SNYK-JAVA-IOKTOR-2980134
  • Email Client > multipart
    • https://github.com/python/cpython/issues/100612

90% of the problems were either a lack of escaping of the " in filename or a lack of escaping of CRLF. Only Ktor had a problem with RFD due to missing escaping of filename* .


Implimentetion example

Spring: https://github.com/spring-projects/spring-framework/blob/4cc91e46b210b4e4e7ed182f93994511391b54ed/spring-web/src/main/java/org/springframework/http/ContentDisposition.java#L259-L267

Symphony: https://github.com/symfony/symfony/blob/123b1651c4a7e219ba59074441badfac65525efe/src/Symfony/Component/Mime/Header/ParameterizedHeader.php#L128-L133

Golang: https://github.com/golang/go/blob/1e7e160d070443147ee38d4de530ce904637a4f3/src/mime/multipart/writer.go#L132-L136


I looked at about 50-80 well-known modules, and about 20% of them retained this vulnerability, so I think it is very significant that clear requirements are stated in the ASVS.

motoyasu-saburi avatar Jan 04 '23 14:01 motoyasu-saburi

As we now have RFC references in requirement texts, we can use it for this one as well.

One candidate for CWE is CWE-641 Improper Restriction of Names for Files and Other Resources. There are no good one as the requirement asks validation+sanitization+encoding.

elarlang avatar Sep 25 '23 18:09 elarlang

I have written a report on this issue. https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714

Also, I have read some RFCs in broad strokes, but the escaping requirement was not clearly stated.

https://datatracker.ietf.org/doc/html/rfc2231
https://datatracker.ietf.org/doc/html/rfc2616
https://datatracker.ietf.org/doc/html/rfc5987
https://datatracker.ietf.org/doc/html/rfc6266
https://datatracker.ietf.org/doc/html/rfc7578
https://datatracker.ietf.org/doc/html/rfc8187

The format of form-data > filename is described in the following RFC.

RFC2616 (obsolete) https://datatracker.ietf.org/doc/html/rfc2616#section-19.5.1

filename-param = "filename" "=" quoted-string

RFC6266 (standard) https://datatracker.ietf.org/doc/html/rfc6266#section-4.1

filename-param = "filename" "=" value
                            | "filename*" "=" ext-value
....
value     = <value, defined in [RFC2616], Section 3.6>
              ; token | quoted-string

I reported a same problem to Scala's Web Framework (Playframework), and the maintainer investigated the issue in considerable detail. https://github.com/playframework/playframework/pull/11571

I hope this will be helpful.

motoyasu-saburi avatar Sep 26 '23 11:09 motoyasu-saburi

that is interesting @motoyasu-saburi

Do you think you could try and formulate a requirement based on the conclusions of your research?

tghosth avatar Sep 26 '23 15:09 tghosth