Skip to content

Commit 662ac3e

Browse files
jkyberneeesjkyberneees
authored
Merge pull request #12 from BackendStack21/fix/path-injection-sidecar-loading
fix(security): validate sidecar paths to prevent path injection attacks
2 parents 4d8bd9f + 5d7a12a commit 662ac3e

File tree

2 files changed

+253
-7
lines changed

2 files changed

+253
-7
lines changed

‎internal/handler/file.go‎

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,9 @@ func (h *FileHandler) serveFromDisk(ctx *fasthttp.RequestCtx, absPath, urlPath s
297297
// compressible and large enough to benefit from compression.
298298
if h.cfg.Compression.Enabled && h.cfg.Compression.Precompressed &&
299299
compress.IsCompressible(ct) && len(data) >= h.cfg.Compression.MinSize {
300-
cached.GzipData = loadSidecar(absPath + ".gz")
301-
cached.BrData = loadSidecar(absPath + ".br")
302-
cached.ZstdData = loadSidecar(absPath + ".zst")
300+
cached.GzipData = h.LoadSidecar(absPath + ".gz")
301+
cached.BrData = h.LoadSidecar(absPath + ".br")
302+
cached.ZstdData = h.LoadSidecar(absPath + ".zst")
303303
}
304304

305305
// Generate on-the-fly gzip if no sidecar and content is compressible.
@@ -501,11 +501,72 @@ func detectContentType(path string, data []byte) string {
501501
return "application/octet-stream"
502502
}
503503

504-
// loadSidecar attempts to read a pre-compressed sidecar file.
505-
// Returns nil if the sidecar does not exist or cannot be read.
506-
func loadSidecar(path string) []byte {
507-
data, err := os.ReadFile(path)
504+
// ValidateSidecarPath validates that a sidecar file path is within the root directory.
505+
// It uses filepath.Clean() to normalize the path (recognized by CodeQL as a sanitizer),
506+
// then verifies the canonical path remains within the root via symlink resolution and
507+
// prefix checking. Returns the validated path or an error if validation fails.
508+
// This function is designed to be recognized by static analyzers as a path sanitizer.
509+
func (h *FileHandler) ValidateSidecarPath(sidecarPath string) (string, error) {
510+
// Step 1: Normalize the path using filepath.Clean().
511+
// This removes ".." and "." components and is recognized by CodeQL as a sanitizer.
512+
cleanPath := filepath.Clean(sidecarPath)
513+
514+
// Step 2: Ensure the path is absolute (relative to root).
515+
// If it's relative, make it absolute relative to root.
516+
var absPath string
517+
if filepath.IsAbs(cleanPath) {
518+
absPath = cleanPath
519+
} else {
520+
absPath = filepath.Join(h.absRoot, cleanPath)
521+
}
522+
523+
// Step 3: Resolve symlinks to get the canonical path.
524+
// This prevents symlink escape attacks where a sidecar could point outside root.
525+
realPath, err := filepath.EvalSymlinks(absPath)
526+
if err != nil {
527+
// File doesn't exist or can't be resolved — return error.
528+
return "", err
529+
}
530+
531+
// Step 4: Resolve the root directory to its canonical path for comparison.
532+
// This is important on platforms like macOS where /tmp → /private/tmp.
533+
realRoot := h.absRoot
534+
if r, err := filepath.EvalSymlinks(h.absRoot); err == nil {
535+
realRoot = r
536+
}
537+
538+
// Step 5: Verify the resolved path is still within the root directory.
539+
// Add a trailing separator to prevent prefix collisions like "/root" matching "/rootsuffix".
540+
rootWithSep := realRoot
541+
if !strings.HasSuffix(rootWithSep, string(filepath.Separator)) {
542+
rootWithSep += string(filepath.Separator)
543+
}
544+
545+
// Reject if the sidecar path escapes the root directory.
546+
if realPath != realRoot && !strings.HasPrefix(realPath, rootWithSep) {
547+
// Sidecar path escapes the root — reject it.
548+
return "", fmt.Errorf("sidecar path escapes root directory")
549+
}
550+
551+
return realPath, nil
552+
}
553+
554+
// LoadSidecar attempts to read a pre-compressed sidecar file.
555+
// Returns nil if the sidecar does not exist, cannot be read, or fails validation.
556+
// The path parameter must be constructed from a validated absolute filesystem path
557+
// (e.g., absPath + ".gz") to ensure it remains within the root directory.
558+
func (h *FileHandler) LoadSidecar(path string) []byte {
559+
// Validate the sidecar path to prevent path traversal attacks.
560+
validatedPath, err := h.ValidateSidecarPath(path)
561+
if err != nil {
562+
// Validation failed (symlink escape, doesn't exist, etc.) — return nil.
563+
return nil
564+
}
565+
566+
// Path is validated and safe — read the file.
567+
data, err := os.ReadFile(validatedPath)
508568
if err != nil {
569+
// File doesn't exist or can't be read — return nil.
509570
return nil
510571
}
511572
return data

‎internal/handler/file_test.go‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package handler_test
22

33
import (
4+
"bytes"
45
"io"
56
"log"
67
"os"
@@ -948,3 +949,187 @@ func BenchmarkHandler_CacheHitQuiet(b *testing.B) {
948949
h(ctx)
949950
}
950951
}
952+
953+
// TestValidateSidecarPath tests the path validation logic for sidecar files.
954+
// This test ensures that CodeQL path-injection alerts are properly addressed
955+
// by verifying that filepath.Clean() + symlink resolution + prefix checking
956+
// prevents all known path traversal attacks.
957+
func TestValidateSidecarPath(t *testing.T) {
958+
root := t.TempDir()
959+
cfg := &config.Config{}
960+
cfg.Files.Root = root
961+
cfg.Cache.Enabled = false
962+
c := cache.NewCache(cfg.Cache.MaxBytes)
963+
h := handler.NewFileHandler(cfg, c)
964+
965+
// Create test files
966+
testFile := filepath.Join(root, "test.txt")
967+
if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
968+
t.Fatal(err)
969+
}
970+
971+
// Create a subdirectory with a file
972+
subdir := filepath.Join(root, "subdir")
973+
if err := os.MkdirAll(subdir, 0755); err != nil {
974+
t.Fatal(err)
975+
}
976+
subFile := filepath.Join(subdir, "sub.txt")
977+
if err := os.WriteFile(subFile, []byte("sub"), 0644); err != nil {
978+
t.Fatal(err)
979+
}
980+
981+
tests := []struct {
982+
name string
983+
path string
984+
wantErr bool
985+
desc string
986+
}{
987+
{
988+
name: "valid absolute path",
989+
path: testFile,
990+
wantErr: false,
991+
desc: "Should accept valid absolute path within root",
992+
},
993+
{
994+
name: "valid relative path",
995+
path: "test.txt",
996+
wantErr: false,
997+
desc: "Should accept valid relative path within root",
998+
},
999+
{
1000+
name: "valid subdirectory path",
1001+
path: filepath.Join(root, "subdir", "sub.txt"),
1002+
wantErr: false,
1003+
desc: "Should accept valid path in subdirectory",
1004+
},
1005+
{
1006+
name: "traversal with ..",
1007+
path: filepath.Join(root, "..", "etc", "passwd"),
1008+
wantErr: true,
1009+
desc: "Should reject path traversal with .. components",
1010+
},
1011+
{
1012+
name: "traversal with multiple ..",
1013+
path: filepath.Join(root, "..", "..", "..", "etc", "passwd"),
1014+
wantErr: true,
1015+
desc: "Should reject multiple .. traversal attempts",
1016+
},
1017+
{
1018+
name: "absolute path outside root",
1019+
path: "/etc/passwd",
1020+
wantErr: true,
1021+
desc: "Should reject absolute path outside root",
1022+
},
1023+
{
1024+
name: "nonexistent file",
1025+
path: filepath.Join(root, "nonexistent.txt"),
1026+
wantErr: true,
1027+
desc: "Should reject nonexistent files (EvalSymlinks fails)",
1028+
},
1029+
}
1030+
1031+
for _, tt := range tests {
1032+
t.Run(tt.name, func(t *testing.T) {
1033+
result, err := h.ValidateSidecarPath(tt.path)
1034+
if (err != nil) != tt.wantErr {
1035+
t.Errorf("%s: got error=%v, wantErr=%v", tt.desc, err, tt.wantErr)
1036+
}
1037+
if !tt.wantErr && err == nil {
1038+
// Verify the result is within root
1039+
realRoot := root
1040+
if r, err := filepath.EvalSymlinks(root); err == nil {
1041+
realRoot = r
1042+
}
1043+
if !strings.HasPrefix(result, realRoot) && result != realRoot {
1044+
t.Errorf("%s: result %q is not within root %q", tt.desc, result, realRoot)
1045+
}
1046+
}
1047+
})
1048+
}
1049+
}
1050+
1051+
// TestLoadSidecar tests the sidecar file loading logic.
1052+
// This test verifies that the CodeQL-compliant path validation
1053+
// allows legitimate sidecar files to be loaded while rejecting attacks.
1054+
func TestLoadSidecar(t *testing.T) {
1055+
root := t.TempDir()
1056+
cfg := &config.Config{}
1057+
cfg.Files.Root = root
1058+
cfg.Cache.Enabled = false
1059+
c := cache.NewCache(cfg.Cache.MaxBytes)
1060+
h := handler.NewFileHandler(cfg, c)
1061+
1062+
// Create a test file and its sidecar
1063+
testFile := filepath.Join(root, "test.txt")
1064+
sidecarFile := filepath.Join(root, "test.txt.gz")
1065+
sidecarContent := []byte("compressed data")
1066+
1067+
if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
1068+
t.Fatal(err)
1069+
}
1070+
if err := os.WriteFile(sidecarFile, sidecarContent, 0644); err != nil {
1071+
t.Fatal(err)
1072+
}
1073+
1074+
tests := []struct {
1075+
name string
1076+
path string
1077+
wantNil bool
1078+
desc string
1079+
}{
1080+
{
1081+
name: "valid sidecar",
1082+
path: sidecarFile,
1083+
wantNil: false,
1084+
desc: "Should load valid sidecar file",
1085+
},
1086+
{
1087+
name: "nonexistent sidecar",
1088+
path: filepath.Join(root, "nonexistent.gz"),
1089+
wantNil: true,
1090+
desc: "Should return nil for nonexistent sidecar",
1091+
},
1092+
{
1093+
name: "traversal attempt",
1094+
path: filepath.Join(root, "..", "etc", "passwd"),
1095+
wantNil: true,
1096+
desc: "Should return nil for traversal attempts",
1097+
},
1098+
}
1099+
1100+
for _, tt := range tests {
1101+
t.Run(tt.name, func(t *testing.T) {
1102+
result := h.LoadSidecar(tt.path)
1103+
if (result == nil) != tt.wantNil {
1104+
t.Errorf("%s: got nil=%v, wantNil=%v", tt.desc, result == nil, tt.wantNil)
1105+
}
1106+
if !tt.wantNil && result != nil {
1107+
if !bytes.Equal(result, sidecarContent) {
1108+
t.Errorf("%s: got %q, want %q", tt.desc, result, sidecarContent)
1109+
}
1110+
}
1111+
})
1112+
}
1113+
}
1114+
1115+
// BenchmarkValidateSidecarPath benchmarks the path validation logic.
1116+
func BenchmarkValidateSidecarPath(b *testing.B) {
1117+
root := b.TempDir()
1118+
cfg := &config.Config{}
1119+
cfg.Files.Root = root
1120+
cfg.Cache.Enabled = false
1121+
c := cache.NewCache(cfg.Cache.MaxBytes)
1122+
h := handler.NewFileHandler(cfg, c)
1123+
1124+
// Create a test file
1125+
testFile := filepath.Join(root, "test.txt")
1126+
if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
1127+
b.Fatal(err)
1128+
}
1129+
1130+
b.ResetTimer()
1131+
b.ReportAllocs()
1132+
for i := 0; i < b.N; i++ {
1133+
_, _ = h.ValidateSidecarPath(testFile)
1134+
}
1135+
}

0 commit comments

Comments
 (0)