Ver código fonte

fix(security): redact at source and cap marshal sizes for CodeQL

CodeQL kept flagging the merge logger because taint flowed Password ->
ClientMergeConflict.Old -> log even with a runtime redact helper -- the
analyzer can't prove the branch excludes credentials. Redact at the
source instead: uuid/password/auth/subId now only ever land in the
conflict struct as <redacted> placeholders, so no caller (log or
otherwise) can leak them.

For the ClientWithAttachments marshal overflow alert, replace the
MaxInt-len() arithmetic with explicit per-input size caps (256MB each),
which is the pattern CodeQL's own docs recommend and recognizes.
MHSanaei 21 horas atrás
pai
commit
b36e5e0869
3 arquivos alterados com 11 adições e 25 exclusões
  1. 1 17
      database/db.go
  2. 8 6
      database/model/model.go
  3. 2 2
      web/service/client.go

+ 1 - 17
database/db.go

@@ -48,21 +48,6 @@ func Dialect() string {
 	return db.Dialector.Name()
 }
 
-var sensitiveConflictFields = map[string]struct{}{
-	"uuid":     {},
-	"password": {},
-	"auth":     {},
-	"subId":    {},
-}
-
-// redactConflictValues masks values for credential-bearing merge fields so
-// they never reach plain-text logs. Non-sensitive fields pass through.
-func redactConflictValues(x model.ClientMergeConflict) (oldV, newV, keptV any) {
-	if _, sensitive := sensitiveConflictFields[x.Field]; sensitive {
-		return "<redacted>", "<redacted>", "<redacted>"
-	}
-	return x.Old, x.New, x.Kept
-}
 
 const (
 	defaultUsername = "admin"
@@ -265,9 +250,8 @@ func seedClientsFromInboundJSON() error {
 				} else {
 					conflicts := model.MergeClientRecord(row, incoming)
 					for _, x := range conflicts {
-						oldV, newV, keptV := redactConflictValues(x)
 						log.Printf("client merge: email=%s conflict on %s old=%v new=%v kept=%v",
-							email, x.Field, oldV, newV, keptV)
+							email, x.Field, x.Old, x.New, x.Kept)
 					}
 					if err := tx.Save(row).Error; err != nil {
 						return err

+ 8 - 6
database/model/model.go

@@ -464,28 +464,30 @@ func MergeClientRecord(existing *ClientRecord, incoming *ClientRecord) []ClientM
 	keep := func(field string, oldV, newV, kept any) {
 		conflicts = append(conflicts, ClientMergeConflict{Field: field, Old: oldV, New: newV, Kept: kept})
 	}
+	const redacted = "<redacted>"
+	keepSecret := func(field string) {
+		conflicts = append(conflicts, ClientMergeConflict{Field: field, Old: redacted, New: redacted, Kept: redacted})
+	}
 
 	incomingNewer := incoming.UpdatedAt > existing.UpdatedAt ||
 		(incoming.UpdatedAt == existing.UpdatedAt && incoming.CreatedAt > existing.CreatedAt)
 
 	if existing.UUID != incoming.UUID && incoming.UUID != "" {
 		if incomingNewer || existing.UUID == "" {
-			keep("uuid", existing.UUID, incoming.UUID, incoming.UUID)
 			existing.UUID = incoming.UUID
-		} else {
-			keep("uuid", existing.UUID, incoming.UUID, existing.UUID)
 		}
+		keepSecret("uuid")
 	}
 	if existing.Password != incoming.Password && incoming.Password != "" {
 		if incomingNewer || existing.Password == "" {
-			keep("password", existing.Password, incoming.Password, incoming.Password)
 			existing.Password = incoming.Password
+			keepSecret("password")
 		}
 	}
 	if existing.Auth != incoming.Auth && incoming.Auth != "" {
 		if incomingNewer || existing.Auth == "" {
-			keep("auth", existing.Auth, incoming.Auth, incoming.Auth)
 			existing.Auth = incoming.Auth
+			keepSecret("auth")
 		}
 	}
 	if existing.Flow != incoming.Flow && incoming.Flow != "" {
@@ -502,8 +504,8 @@ func MergeClientRecord(existing *ClientRecord, incoming *ClientRecord) []ClientM
 	}
 	if existing.SubID != incoming.SubID && incoming.SubID != "" {
 		if incomingNewer || existing.SubID == "" {
-			keep("subId", existing.SubID, incoming.SubID, incoming.SubID)
 			existing.SubID = incoming.SubID
+			keepSecret("subId")
 		}
 	}
 	if existing.TotalGB != incoming.TotalGB {

+ 2 - 2
web/service/client.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"math"
 	"strings"
 	"sync"
 	"time"
@@ -48,7 +47,8 @@ func (c ClientWithAttachments) MarshalJSON() ([]byte, error) {
 	if len(rec) < 2 || rec[len(rec)-1] != '}' || len(extra) <= 2 {
 		return rec, nil
 	}
-	if len(extra) > math.MaxInt-len(rec) {
+	const maxMarshalSize = 256 << 20
+	if len(rec) > maxMarshalSize || len(extra) > maxMarshalSize {
 		return rec, nil
 	}
 	out := make([]byte, 0, len(rec)+len(extra))