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

Path Traversal Attacks on Windows #1691

Open
egovorukhin opened this issue Jan 10, 2024 · 7 comments
Open

Path Traversal Attacks on Windows #1691

egovorukhin opened this issue Jan 10, 2024 · 7 comments

Comments

@egovorukhin
Copy link

Hello, I found another bug security on windows.

example - curl http://localhost:8081/api/\../\../\../\../\../\../\../\../windows/win.ini -k

SOLUTION

file strings.go

var (
   ...
  strBackSlashDotDotSlash = []byte(`\../`)
  ...
)

file uri.go

func normalizePath(dst, src []byte) []byte {
...
if filepath.Separator == '\\' {
...
        // remove /foo\../ parts
	for {
		n := bytes.Index(b, strBackSlashDotDotSlash)
		if n < 0 {
			break
		}
		nn := bytes.LastIndexByte(b[:n], '/')
		if nn < 0 {
			nn = 0
		}
		n += len(strBackSlashDotDotSlash) - 1
		copy(b[nn:], b[n:])
		b = b[:len(b)-n+nn]
	}
...
}
@erikdubbelboer
Copy link
Collaborator

Can you make a pull request including a test that fails before and passes after your change?

@egovorukhin
Copy link
Author

#1698 - created pull request.

How do you think about changing the function (normalizePath) more? Please check the changes

@gaby
Copy link
Contributor

gaby commented Jan 31, 2024

@egovorukhin @erikdubbelboer This was supposed to be reported using the process here: https://github.com/valyala/fasthttp/security

This could qualify for a CVE, and it was reported improperly.

@erikdubbelboer
Copy link
Collaborator

I think we should just disable FS on windows. It's not safe, it's not properly tested and nobody should be using it in production.

@egovorukhin
Copy link
Author

Let's replace backslash with slash when used in windows, then a check for "path traversal" will occur. What do you think about this?

file - uri.go
func - normalizePath

if filepath.Separator == '\\' {
	dst = replaceSlashes(dst)
}

https://github.com/egovorukhin/fasthttp/blob/4d48887eb813b0f1caac0217751888897f601d50/uri.go#L579

file - uri_windows.go

func replaceSlashes(dst []byte) []byte {
	for i := range dst {
		if dst[i] == '\\' {
			dst[i] = '/'
		}
	}
	return dst
}

https://github.com/egovorukhin/fasthttp/blob/4d48887eb813b0f1caac0217751888897f601d50/uri_windows.go#L13

@erikdubbelboer
Copy link
Collaborator

I don't think that will fix it. How about we just reject all requests with .. on windows?

@egovorukhin
Copy link
Author

Ok, let's do this. Do you have a solution?

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

3 participants