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

file.Filename should not be trusted. There should be a sanitize function, or give a warning in docs. #1693

Closed
ganlvtech opened this issue Dec 11, 2018 · 2 comments

Comments

@ganlvtech
Copy link
Contributor

commented Dec 11, 2018

  • go version: 1.11
  • operating system: Windows 10 64bit

Description

if err := c.SaveUploadedFile(file, file.Filename); err != nil {

file, _ := c.FormFile("file")
c.SaveUploadedFile(file, file.Filename)

We must not trust user input file.Filename!

Reproduce

First, start examples/upload-file/single/main.go server.

cd ~/go/src/github.com/gin-gonic/gin/examples/upload-file/single
go run main.go

Start a new terminal and upload a file (such as the main.go itself) with cURL.

curl -X POST -F 'file=@main.go; filename=../main.go' http://127.0.0.1:8080/upload

Then, you will find the uploaded file is at ~/go/src/github.com/gin-gonic/gin/examples/upload-file/main.go. Upload a file to parent dir is really dangerous.

I don't know if it's by design. But I think, at least, there should be a warning asking developers to sanitize the input properly.

Solution

The simplest way may be

import "path/filepath"

file, _ := c.FormFile("file")
filename := filepath.Base(file.Filename)
c.SaveUploadedFile(file, filename)

This will restrict the upload file to current directory.

@ganlvtech ganlvtech changed the title file.Filename must not be trusted. There should be a sanitize function. give a warning in docs. file.Filename should not be trusted. There should be a sanitize function, or give a warning in docs. Dec 12, 2018
@thinkerou

This comment has been minimized.

Copy link
Member

commented Dec 15, 2018

@ganlvtech please commit pull request, thanks!

@thinkerou thinkerou closed this in 1542eff Dec 17, 2018
@thinkerou

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

#1699 merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.