security: add escape to arbitrary file access

[dduzgun-security]

Resolves Arbitrary file write extracting an archive containing symbolic links alerts by using the escape function.

Ref: NET-9781 and NET-9776

[tgross]

@dduzgun-security this looks good. It'd be nice if we could add some test coverage to these checks, similar to what we have in TestPrevAlloc_StreamAllocDir_Error.

[dduzgun-security]

Looks like other tests broke, working to fix them.

=== Failed
=== FAIL: client/allocwatcher TestPrevAlloc_StreamAllocDir_BadSymlink (0.00s)
    assert.go:14: 
        alloc_watcher_unix_test.go:115: expected error; got nil

=== FAIL: client/allocwatcher TestPrevAlloc_StreamAllocDir_BadSymlink (re-run 1) (0.00s)
    assert.go:14: 
        alloc_watcher_unix_test.go:115: expected error; got nil

=== FAIL: client/allocwatcher TestPrevAlloc_StreamAllocDir_BadSymlink (re-run 2) (0.00s)
    assert.go:14: 
        alloc_watcher_unix_test.go:115: expected error; got nil

=== FAIL: client/allocwatcher TestPrevAlloc_StreamAllocDir_BadSymlink (re-run 3) (0.00s)
    assert.go:14: 
        alloc_watcher_unix_test.go:115: expected error; got nil

[dduzgun-security]

🤔 hmm, running the tests locally works but the CI seems to fail, do we have some sort of caching that may cause that?
Or maybe I may be missing something

Local test results (on MacOS M2 23.5.0 Darwin Kernel Version 23.5.0 arm64)👇

Running tool: /opt/homebrew/bin/go test -timeout 30s -run ^TestPrevAlloc_StreamAllocDir_BadSymlink$ github.com/hashicorp/nomad/client/allocwatcher

=== RUN   TestPrevAlloc_StreamAllocDir_BadSymlink
=== PAUSE TestPrevAlloc_StreamAllocDir_BadSymlink
=== CONT  TestPrevAlloc_StreamAllocDir_BadSymlink
2024-06-17T10:05:52.743-0400 [DEBUG] .../hashicorp/nomad/client/allocwatcher/allocwatcher/alloc_watcher.go:555: streaming snapshot of previous alloc: destination=/var/folders/th/2vhwms5n1qbf8rgl7cy7902m0000gn/T/TestPrevAlloc_StreamAllocDir_BadSymlink796392596/001
--- PASS: TestPrevAlloc_StreamAllocDir_BadSymlink (0.00s)
PASS
ok      github.com/hashicorp/nomad/client/allocwatcher  0.342s

[tgross]

@dduzgun-security this is what I'm getting locally with this branch checked out:

$ go test -count=1 -run ^TestPrevAlloc_StreamAllocDir_BadSymlink$ github.com/hashicorp/nomad/client/allocwatcher
2024-06-17T12:31:56.462-0400 [DEBUG] allocwatcher/alloc_watcher.go:555: streaming snapshot of previous alloc: destination=/tmp/TestPrevAlloc_StreamAllocDir_BadSymlink4141846833/001
--- FAIL: TestPrevAlloc_StreamAllocDir_BadSymlink (0.00s)
    assert.go:14:
        alloc_watcher_unix_test.go:114: expected error; got nil
FAIL
FAIL    github.com/hashicorp/nomad/client/allocwatcher  0.018s
FAIL

Looks like you're testing on macOS though and I'm testing on Linux (as is CI); might be worth checking that there isn't OS-specific behavior here.

[dduzgun-security]

When I tried it on an ubuntu VM, the test passed

Maybe something related with the GHA runner 🤔 I'll give it a shot with this container image
https://github.com/actions-runner-controller/actions-runner-controller/pkgs/container/actions-runner-controller%2Factions-runner-dind/226924128?tag=ubuntu-22.04

[tgross]

This is looking good, @dduzgun-security. Just a few more issues to get it ready to ship.

[tgross]

Looking good! Just a few minor items to pick up and this should be good-to-merge

[tgross]

LGTM! Thanks @dduzgun-security!

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.