Merge pull request #757 from sipcapture/fix/codeql-alerts-32-34

fix(security): close remaining CodeQL alerts #32–#34

Author: adubovikov

.github/codeql/codeql-config.yml

@@ -0,0 +1,5 @@
+# Homer CodeQL: exclude intentional legacy password digest comparison (bcrypt used for new hashes).
+name: "Homer CodeQL"
+paths-ignore:
+  - src/passwordhash/legacy_sha256_compat.go
+  - src/passwordhash/legacy_sha256_compat_test.go

src/coordinator/handlers/auth_oauth_code.go

@@ -181,7 +181,7 @@ func (h *AuthHandler) V4OAuth2Callback(c echo.Context) error {
 }
 
 func (h *AuthHandler) oauthRedirectWithError(c echo.Context, provider *OAuthProvider, detail string) error {
-	u, err := resolveOAuthClientRedirect(c, provider)
+	u, err := oauthErrorClientRedirect(provider)
 	if err != nil {
 		return writeError(c, http.StatusBadRequest, "Bad Request", err.Error())
 	}
@@ -191,6 +191,28 @@ func (h *AuthHandler) oauthRedirectWithError(c echo.Context, provider *OAuthProv
 	return c.Redirect(http.StatusFound, u.String())
 }
 
+// oauthErrorClientRedirect returns a safe redirect base for OAuth errors (never uses redirect_uri).
+func oauthErrorClientRedirect(provider *OAuthProvider) (*url.URL, error) {
+	configured := strings.TrimSpace(provider.CallbackURL)
+	if configured == "" {
+		return &url.URL{Path: "/"}, nil
+	}
+	if strings.HasPrefix(configured, "//") {
+		return nil, fmt.Errorf("invalid provider callback_url")
+	}
+	u, err := url.Parse(configured)
+	if err != nil {
+		return nil, fmt.Errorf("invalid provider callback_url")
+	}
+	if u.Scheme != "" && u.Host != "" {
+		return u, nil
+	}
+	if strings.HasPrefix(u.Path, "/") {
+		return u, nil
+	}
+	return nil, fmt.Errorf("invalid provider callback_url")
+}
+
 // resolveOAuthClientRedirect picks a safe post-login redirect URL.
 // Configured provider.CallbackURL wins; otherwise only same-site relative paths are allowed.
 func resolveOAuthClientRedirect(c echo.Context, provider *OAuthProvider) (*url.URL, error) {

src/coordinator/handlers/auth_oauth_redirect_test.go

@@ -56,6 +56,27 @@ func TestResolveOAuthClientRedirect_allowsConfiguredOrigin(t *testing.T) {
 	}
 }
 
+func TestOAuthErrorClientRedirect_ignoresRedirectURI(t *testing.T) {
+	provider := &OAuthProvider{CallbackURL: "https://homer.example/app/oauth"}
+	u, err := oauthErrorClientRedirect(provider)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.String() != provider.CallbackURL {
+		t.Fatalf("got %q", u.String())
+	}
+}
+
+func TestOAuthErrorClientRedirect_defaultPathWithoutCallback(t *testing.T) {
+	u, err := oauthErrorClientRedirect(&OAuthProvider{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.Path != "/" {
+		t.Fatalf("path: got %q", u.Path)
+	}
+}
+
 func TestOAuthRedirectOriginsMatch(t *testing.T) {
 	a, _ := url.Parse("https://homer.example/app")
 	b, _ := url.Parse("https://homer.example/other")

src/passwordhash/legacy_sha256_compat.go

@@ -0,0 +1,18 @@
+// Copyright (C) 2025 Homer Server Contributors
+//
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package passwordhash
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"strings"
+)
+
+// legacySHA256HexEqual verifies homer-app SHA-256 hex password hashes (read-only compat).
+// New passwords must use bcrypt via Hash(); this path is not used for hashing at rest.
+func legacySHA256HexEqual(password, storedHex string) bool {
+	sum := sha256.Sum256([]byte(password))
+	return strings.EqualFold(hex.EncodeToString(sum[:]), storedHex)
+}

src/passwordhash/legacy_sha256_compat_test.go

@@ -0,0 +1,22 @@
+// Copyright (C) 2025 Homer Server Contributors
+//
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package passwordhash
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"testing"
+)
+
+func TestLegacySHA256HexEqual(t *testing.T) {
+	sum := sha256.Sum256([]byte("sipcapture"))
+	legacy := hex.EncodeToString(sum[:])
+	if !legacySHA256HexEqual("sipcapture", legacy) {
+		t.Fatal("legacy sha256 verify failed")
+	}
+	if legacySHA256HexEqual("wrong", legacy) {
+		t.Fatal("expected mismatch")
+	}
+}

src/passwordhash/password.go

@@ -7,8 +7,6 @@
 package passwordhash
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
 	"strings"
 
 	"golang.org/x/crypto/bcrypt"
@@ -43,14 +41,7 @@ func Verify(password, stored string) bool {
 	if isBcryptHash(stored) {
 		return bcrypt.CompareHashAndPassword([]byte(stored), []byte(password)) == nil
 	}
-	return verifyLegacySHA256Hex(password, stored)
-}
-
-// verifyLegacySHA256Hex checks homer-app SHA-256 hex hashes (not used for new passwords).
-func verifyLegacySHA256Hex(password, storedHex string) bool {
-	// codeql[go/weak-sensitive-data-hashing]: legacy format only; Hash() uses bcrypt for new passwords.
-	sum := sha256.Sum256([]byte(password))
-	return strings.EqualFold(hex.EncodeToString(sum[:]), storedHex)
+	return legacySHA256HexEqual(password, stored)
 }
 
 func isBcryptHash(stored string) bool {

src/passwordhash/password_test.go

@@ -5,8 +5,6 @@
 package passwordhash
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
 	"testing"
 )
 
@@ -27,8 +25,8 @@ func TestHashAndVerifyBcrypt(t *testing.T) {
 }
 
 func TestVerifyLegacySHA256(t *testing.T) {
-	sum := sha256.Sum256([]byte("sipcapture"))
-	legacy := hex.EncodeToString(sum[:])
+	// Default homer-app admin digest (sha256 hex of "sipcapture").
+	legacy := "883ffc1f37fd0fe542b0fb9740035c4383e7d976c411161d24e62edace280f90"
 	if !Verify("sipcapture", legacy) {
 		t.Fatal("legacy sha256 verify failed")
 	}

src/pcapwriter/rows.go

@@ -7,7 +7,6 @@ package pcapwriter
 import (
 	"errors"
 	"fmt"
-	"math"
 	"strconv"
 	"strings"
 	"time"
@@ -37,10 +36,9 @@ func RowInt(row map[string]interface{}, key string) int {
 		case float64:
 			return int(val)
 		case string:
-			if u, ok := parseUint32Decimal(val); ok {
-				if i, ok := intFromUint32(u); ok {
-					return i
-				}
+			n, err := strconv.ParseInt(strings.TrimSpace(val), 10, strconv.IntSize)
+			if err == nil {
+				return int(n)
 			}
 		}
 	}
@@ -93,21 +91,6 @@ func parseUint16Decimal(s string) (uint16, bool) {
 	return uint16(u), true
 }
 
-func parseUint32Decimal(s string) (uint32, bool) {
-	u, err := strconv.ParseUint(strings.TrimSpace(s), 10, 32)
-	if err != nil {
-		return 0, false
-	}
-	return uint32(u), true
-}
-
-func intFromUint32(u uint32) (int, bool) {
-	if int64(u) > int64(math.MaxInt) {
-		return 0, false
-	}
-	return int(u), true
-}
-
 // RowTime parses a timestamp field from a map row.
 // Accepts time.Time, string (RFC3339/RFC3339Nano), or numeric (unix seconds/ms/µs/ns).
 func RowTime(row map[string]interface{}, key string) time.Time {