fix some CodeQL warnings (#6029)

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

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")
+}

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
 		}

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)
 }