fasthttp icon indicating copy to clipboard operation
fasthttp copied to clipboard

Path Traversal Attacks on Windows

Open egovorukhin opened this issue 1 year ago • 7 comments

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]
	}
...
}

egovorukhin avatar Jan 10 '24 05:01 egovorukhin

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

erikdubbelboer avatar Jan 14 '24 01:01 erikdubbelboer

https://github.com/valyala/fasthttp/pull/1698 - created pull request.

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

egovorukhin avatar Jan 19 '24 08:01 egovorukhin

@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.

gaby avatar Jan 31 '24 04:01 gaby

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.

erikdubbelboer avatar Jan 31 '24 23:01 erikdubbelboer

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

egovorukhin avatar Feb 01 '24 05:02 egovorukhin

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

erikdubbelboer avatar Feb 10 '24 09:02 erikdubbelboer

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

egovorukhin avatar Feb 13 '24 05:02 egovorukhin