Skip to content

Commit

Permalink
loader: Block reading of UNC paths
Browse files Browse the repository at this point in the history
If a UNC path is provided to OPA it won't read it
and instead return an error. This applies to paths
to load bundles and individual data/policy files.

One reason behind blocking UNC paths is they could
trigger a NTLMv2 hash leak. For example, if a SMB share
is provided, OPA will attempt to open it triggering LLMNR
queries which contain the client's NTLMv2 hash which can be cracked
using some tools. This could be exploited by a malicious user.

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
  • Loading branch information
ashutosh-narkar committed Aug 16, 2024
1 parent 14ff052 commit ebca544
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 0 deletions.
31 changes: 31 additions & 0 deletions loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,18 @@ func (fl fileLoader) AsBundle(path string) (*bundle.Bundle, error) {
return nil, err
}

if err := checkForUNCPath(path); err != nil {
return nil, err
}

var bundleLoader bundle.DirectoryLoader
var isDir bool
if fl.reader != nil {
bundleLoader = bundle.NewTarballLoaderWithBaseURL(fl.reader, path).WithFilter(fl.filter)
} else {
bundleLoader, isDir, err = GetBundleDirectoryLoaderFS(fl.fsys, path, fl.filter)
}

if err != nil {
return nil, err
}
Expand Down Expand Up @@ -303,6 +308,10 @@ func GetBundleDirectoryLoaderFS(fsys fs.FS, path string, filter Filter) (bundle.
return nil, false, err
}

if err := checkForUNCPath(path); err != nil {
return nil, false, err
}

var fi fs.FileInfo
if fsys != nil {
fi, err = fs.Stat(fsys, path)
Expand Down Expand Up @@ -663,12 +672,18 @@ func allRec(fsys fs.FS, path string, filter Filter, errors *Errors, loaded *Resu
return
}

if err := checkForUNCPath(path); err != nil {
errors.add(err)
return
}

var info fs.FileInfo
if fsys != nil {
info, err = fs.Stat(fsys, path)
} else {
info, err = os.Stat(path)
}

if err != nil {
errors.add(err)
return
Expand Down Expand Up @@ -804,3 +819,19 @@ func makeDir(path []string, x interface{}) (map[string]interface{}, bool) {
}
return makeDir(path[:len(path)-1], map[string]interface{}{path[len(path)-1]: x})
}

// isUNC reports whether path is a UNC path.
func isUNC(path string) bool {
return len(path) > 1 && isSlash(path[0]) && isSlash(path[1])
}

func isSlash(c uint8) bool {
return c == '\\' || c == '/'
}

func checkForUNCPath(path string) error {
if isUNC(path) {
return fmt.Errorf("UNC path read is not allowed")
}
return nil
}
56 changes: 56 additions & 0 deletions loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"embed"
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -574,6 +575,61 @@ func TestAsBundleWithFile(t *testing.T) {
})
}

func TestCheckForUNCPath(t *testing.T) {
cases := []struct {
input string
wantErr bool
err error
}{
{
input: "c:/foo",
wantErr: false,
},
{
input: "file:///c:/a/b",
wantErr: false,
},
{
input: `\\localhost\c$`,
wantErr: true,
err: fmt.Errorf("UNC path read is not allowed"),
},
{
input: `\\\\localhost\c$`,
wantErr: true,
err: fmt.Errorf("UNC path read is not allowed"),
},
{
input: `//localhost/foo`,
wantErr: true,
err: fmt.Errorf("UNC path read is not allowed"),
},
{
input: `file:///a/b/c`,
wantErr: false,
},
}

for _, tc := range cases {
t.Run(tc.input, func(t *testing.T) {
err := checkForUNCPath(tc.input)
if tc.wantErr {
if err == nil {
t.Fatal("Expected error but got nil")
}

if tc.err != nil && tc.err.Error() != err.Error() {
t.Fatalf("Expected error message %v but got %v", tc.err.Error(), err.Error())
}
} else {
if err != nil {
t.Fatalf("Unexpected error %v", err)
}
}
})
}
}

func TestLoadRooted(t *testing.T) {
files := map[string]string{
"/foo.json": "[1,2,3]",
Expand Down

0 comments on commit ebca544

Please sign in to comment.