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 #1226

Closed
egovorukhin opened this issue Feb 21, 2022 · 7 comments
Closed

Path Traversal Attacks #1226

egovorukhin opened this issue Feb 21, 2022 · 7 comments

Comments

@egovorukhin
Copy link

Hello, I found a problem when requesting - path traversal attacks (https://localhost/..%5clogs/app.log). If you specify a backslash (%5c) character in the path, then you can follow the path /../ and get data from the root. It may be worth adding a check for part of the path - /... strSlashDotDotBackSlash = []byte("/.."). At your discretion. Thanks.

@erikdubbelboer
Copy link
Collaborator

I'm guessing this is about the unsafe usage of one of the ServeFile functions? They allow anything as they use / as root for all the requests. Using user specified paths for these functions is very unsafe, I had added some documentation changes for that here: #1228

Or was this about another issue?

@egovorukhin
Copy link
Author

I wrote an example for demonstration https://github.com/egovorukhin/pathTraversalAttacks, using fiber(https://github.com/gofiber/fiber). Checking for the correctness of the path in the function fasthttp->uri.go->normalizePath(dst, src []byte) []byte.

@erikdubbelboer
Copy link
Collaborator

I'm not seeing any issues with your example repo:

% git clone git@github.com:egovorukhin/pathTraversalAttacks.git
% go mod vendor
% go run main.go &
% curl 'http://localhost:3003/..%5csecret.txt'
Cannot GET /..%5csecret.txt
% curl 'http://localhost:3003/..%5c..%5clogs/secret.txt'
Cannot GET /..%5c..%5clogs/secret.txt
% curl 'http://localhost:3003/..%5cclogs/secret.txt'
Cannot GET /..%5cclogs/secret.txt
% curl 'http://localhost:3003/home'
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Title</title>
</head>
<body>
  <h1>Hello world!</h1>
</body>
</html>

Are you maybe on a Windows machine? Does running these commands result in something different for you?

@egovorukhin
Copy link
Author

Yes, app run on a Windows Cluster. Could you add a fix for such cases?! Please.

@erikdubbelboer
Copy link
Collaborator

fasthttp.FS is completely incompatible with Windows. See #1108 and #1101.

Now that you have shown that it's also not secure on Windows I'm wondering if I should prevent the use of fasthttp.FS on windows by throwing an error. Either that or someone needs to take the time to make fasthttp.FS compatible with windows.

@egovorukhin
Copy link
Author

I added the code to the normalizePath function and it solved my problem

file strings.go

var (
   ...
  strSlashDotDotBackSlash = []byte(`/..\`)
  strBackSlashDotDotBackSlash = []byte(`\..\`)
  ...
)

file uri.go

func normalizePath(dst, src []byte) []byte {
...

// remove /foo/..\ parts
	for {
		n := bytes.Index(b, strSlashDotDotBackSlash)
		if n < 0 {
			break
		}
		nn := bytes.LastIndexByte(b[:n], '/')
		if nn < 0 {
			nn = 0
		}
		n += len(strSlashDotDotBackSlash) - 1
		copy(b[nn:], b[n:])
		b = b[:len(b)-n+nn]
	}

	// remove /foo\..\ parts
	for {
		n := bytes.Index(b, strBackSlashDotDotBackSlash)
		if n < 0 {
			break
		}
		nn := bytes.LastIndexByte(b[:n], '/')
		if nn < 0 {
			nn = 0
		}
		n += len(strBackSlashDotDotBackSlash) - 1
		copy(b[nn:], b[n:])
		b = b[:len(b)-n+nn]
	}

...
}

@egovorukhin
Copy link
Author

egovorukhin commented Jan 9, 2024

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

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

No branches or pull requests

2 participants