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

ganlvtech 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 '[email protected]; filename=../main.go' http:https://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
Copy link
Member

thinkerou commented Dec 15, 2018

@ganlvtech please commit pull request, thanks!

@thinkerou
Copy link
Member

#1699 merged!

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

No branches or pull requests

2 participants