From c8d0e47488d84d084f6ff2bc59821648682cdb9f Mon Sep 17 00:00:00 2001 From: Alessandro Ros Date: Mon, 3 Aug 2026 19:23:08 +0200 Subject: [PATCH] fix some CodeQL warnings (#6029) --- internal/api/api_recordings.go | 16 ++++++++-------- internal/api/api_recordings_test.go | 15 +++++++++++++++ internal/servers/hls/muxer_instance.go | 6 +++--- internal/servers/hls/server_test.go | 8 ++++---- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/internal/api/api_recordings.go b/internal/api/api_recordings.go index fdfeae3e..1c4f3ad3 100644 --- a/internal/api/api_recordings.go +++ b/internal/api/api_recordings.go @@ -16,22 +16,22 @@ import ( // this prevents directory traversal. // functionally it's useless since there's already conf.IsValidPathName, but it's needed by CodeQL. -func isSubdirectoryOf(base string, path string) error { +func absolutePathInside(base string, candidate string) (string, error) { baseAbs, err := filepath.Abs(filepath.Clean(base)) if err != nil { - return err + return "", err } - candidateAbs, err := filepath.Abs(filepath.Join(baseAbs, path)) + candidateAbs, err := filepath.Abs(filepath.Clean(candidate)) if err != nil { - return err + return "", err } if !strings.HasPrefix(candidateAbs, baseAbs) { - return fmt.Errorf("path escapes base directory") + return "", fmt.Errorf("path escapes base directory") } - return nil + return candidateAbs, nil } func recordingsOfPath( @@ -128,7 +128,7 @@ func (a *API) onRecordingDeleteSegment(ctx *gin.Context) { pathConf.RecordFormat, ) - err = isSubdirectoryOf(commonPath, pathFormat) + pathFormat, err = absolutePathInside(commonPath, pathFormat) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -138,7 +138,7 @@ func (a *API) onRecordingDeleteSegment(ctx *gin.Context) { Start: start, }.Encode(pathFormat) - err = isSubdirectoryOf(pathFormat, segmentPath) + segmentPath, err = absolutePathInside(commonPath, segmentPath) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return diff --git a/internal/api/api_recordings_test.go b/internal/api/api_recordings_test.go index 7f0a1bcb..3bdb1008 100644 --- a/internal/api/api_recordings_test.go +++ b/internal/api/api_recordings_test.go @@ -1,6 +1,7 @@ package api //nolint:revive import ( + "fmt" "net/http" "net/url" "os" @@ -263,3 +264,17 @@ func TestRecordingsSegmentGetInvalidPath(t *testing.T) { require.Equal(t, http.StatusBadRequest, resp.StatusCode) } + +func TestAbsolutePathInside(t *testing.T) { + base := t.TempDir() + + inside, err := absolutePathInside(base, filepath.Join(base, "sub", "file.mp4")) + require.NoError(t, err) + require.Equal(t, filepath.Join(base, "sub", "file.mp4"), inside) + + _, err = absolutePathInside(base, filepath.Join(base, "..", "escape.mp4")) + require.EqualError(t, err, "path escapes base directory") + + _, err = absolutePathInside(base, fmt.Sprintf("%s-sibling/../file.mp4", base)) + require.EqualError(t, err, "path escapes base directory") +} diff --git a/internal/servers/hls/muxer_instance.go b/internal/servers/hls/muxer_instance.go index 299a7b62..eb221ac4 100644 --- a/internal/servers/hls/muxer_instance.go +++ b/internal/servers/hls/muxer_instance.go @@ -28,13 +28,13 @@ const ( // this prevents directory traversal. // functionally it's useless since there's already conf.IsValidPathName, but it's needed by CodeQL. -func safeSubDirectory(base string, pathName string) (string, error) { +func absolutePathInside(base string, candidate string) (string, error) { baseAbs, err := filepath.Abs(filepath.Clean(base)) if err != nil { return "", err } - candidateAbs, err := filepath.Abs(filepath.Join(baseAbs, pathName)) + candidateAbs, err := filepath.Abs(filepath.Clean(candidate)) if err != nil { return "", err } @@ -78,7 +78,7 @@ func (mi *muxerInstance) initialize() error { if mi.directory != "" { var err error - muxerDirectory, err = safeSubDirectory(mi.directory, mi.pathName) + muxerDirectory, err = absolutePathInside(mi.directory, filepath.Join(mi.directory, mi.pathName)) if err != nil { return err } diff --git a/internal/servers/hls/server_test.go b/internal/servers/hls/server_test.go index 75a3d818..2f4ed5ce 100644 --- a/internal/servers/hls/server_test.go +++ b/internal/servers/hls/server_test.go @@ -538,17 +538,17 @@ func TestServerDirectory(t *testing.T) { require.NoError(t, err) } -func TestSafeSubDirectory(t *testing.T) { +func TestAbsolutePathInside(t *testing.T) { base := t.TempDir() - path, err := safeSubDirectory(base, "group/cam1") + path, err := absolutePathInside(base, filepath.Join(base, "group", "cam1")) require.NoError(t, err) require.Equal(t, filepath.Join(base, "group", "cam1"), path) - _, err = safeSubDirectory(base, "../cam1") + _, err = absolutePathInside(base, filepath.Join(base, "..", "cam1")) require.Error(t, err) - _, err = safeSubDirectory(base, "group/../../cam1") + _, err = absolutePathInside(base, filepath.Join(base, "group", "..", "..", "cam1")) require.Error(t, err) }