Path Traversal Attacks - Githubissues

valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http

MIT License

21.67k stars 1.75k forks source link

Path Traversal Attacks #1226

Closed egovorukhin closed 2 years ago

egovorukhin commented 2 years ago

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 commented 2 years ago

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: https://github.com/valyala/fasthttp/pull/1228

Or was this about another issue?

egovorukhin commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

erikdubbelboer commented 2 years ago

fasthttp.FS is completely incompatible with Windows. See https://github.com/valyala/fasthttp/pull/1108 and https://github.com/valyala/fasthttp/issues/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 commented 2 years ago

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 commented 8 months ago

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