Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe attachments can corrupt user's local files #2531

Closed
3 tasks
hktalent opened this issue Oct 14, 2023 · 1 comment · Fixed by #2541
Closed
3 tasks

Unsafe attachments can corrupt user's local files #2531

hktalent opened this issue Oct 14, 2023 · 1 comment · Fixed by #2541

Comments

@hktalent
Copy link

Issue Description

Return Response Header:
HTTP/1.1 304 Not Modified
Content-Disposition: attachment; filename="malicious.sh";dummy=.txt"
Last-Modified: Tue, 19 Jul 2022 08:53:26 GMT
Date: Wed, 20 Jul 2022 11:17:43 GMT
Cause & Fix

The first thing to escape is the filename

The second thing, The Filename escape requirements are then as follows

" --> %2F, "
\r --> %0D, \r
\n --> %0A, \n
The third thing,
This line is the process of "escaping the 'file name at download time' on the HTTP Response Header.
In other words, filepath is irrelevant.

(The part that actually reads the file is the following line (filepath) )

echo/context.go

Line 588 in 98a5237

c.response.Header().Set(HeaderContentDisposition, fmt.Sprintf("%s; filename=%q", dispositionType, name))

c.response.Header().Set(HeaderContentDisposition, fmt.Sprintf("%s; filename=%q", dispositionType, name))

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

Actual behaviour

Steps to reproduce

filename := "malicious.sh";dummy=.txt"

Working code to debug

package main

func main() {
}

Version/commit

Reference

RFC 6266 (section 5)
https://tools.ietf.org/html/rfc6266#section-5

What WG - html spec > multipart/form-data :
https://html.spec.whatwg.org/#multipart-form-data

OWASP ASVS (Related issue):
OWASP/ASVS#1390

Golang impliments:
https://github.com/golang/go/blob/e0e0c8fe9881bbbfe689ad94ca5dddbb252e4233/src/mime/multipart/writer.go#L144

Spring (Java) Impliments:
https://github.com/spring-projects/spring-framework/blob/4f8516e2c3ca420b1608840ab901bf9df7e4d5f1/spring-web/src/main/java/org/springframework/http/ContentDisposition.java#L594-L617

Symfony(PHP) Impliments:
https://github.com/symfony/symfony/blob/123b1651c4a7e219ba59074441badfac65525efe/src/Symfony/Component/HttpFoundation/HeaderUtils.php#L187-L189

This is my own article, but it summarizes the impact, etc. on this issue.
https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714

@aldas
Copy link
Contributor

aldas commented Nov 7, 2023

@hktalent Thank you for reporting this.

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

Successfully merging a pull request may close this issue.

2 participants