From e93c8b40ae2fbc3ffea5045293a376c2d60defe4 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sat, 28 Mar 2026 13:33:48 +0000 Subject: [PATCH] Fix CodeQL integer and audit findings --- internal/smartctl/collector.go | 4 +-- pkg/audit/audit.go | 15 ++++++++++- pkg/audit/audit_test.go | 18 ++++++++++++++ pkg/proxmox/client.go | 16 +++++++++--- pkg/proxmox/client_test.go | 23 +++++++++++++++++ pkg/proxmox/intconv.go | 44 +++++++++++++++++++++++++++++++++ pkg/proxmox/replication.go | 43 ++++++++++++++++++-------------- pkg/proxmox/replication_test.go | 5 ++++ 8 files changed, 142 insertions(+), 26 deletions(-) create mode 100644 pkg/proxmox/intconv.go diff --git a/internal/smartctl/collector.go b/internal/smartctl/collector.go index cd96f594a..99010306a 100644 --- a/internal/smartctl/collector.go +++ b/internal/smartctl/collector.go @@ -929,9 +929,9 @@ func parseSMARTOutput(output []byte, target smartctlTarget) (*DiskSMART, error) } else { for _, attr := range smartData.ATASmartAttributes.Table { if attr.ID == 194 || attr.ID == 190 { - temp := int(parseRawValue(attr.Raw.String, attr.Raw.Value)) + temp := parseRawValue(attr.Raw.String, attr.Raw.Value) if temp > 0 && temp < 150 { - result.Temperature = temp + result.Temperature = int(temp) break } } diff --git a/pkg/audit/audit.go b/pkg/audit/audit.go index f54a6b5dc..629cbe024 100644 --- a/pkg/audit/audit.go +++ b/pkg/audit/audit.go @@ -9,6 +9,8 @@ package audit import ( + "crypto/sha256" + "encoding/hex" "sync" "time" @@ -132,6 +134,7 @@ func NewConsoleLogger() *ConsoleLogger { // Log writes an audit event to zerolog. func (c *ConsoleLogger) Log(event Event) error { + detailsPresent, detailsLen, detailsDigest := summarizeAuditDetails(event.Details) logEvent := log.With(). Str("audit_id", event.ID). Str("event", event.EventType). @@ -139,7 +142,9 @@ func (c *ConsoleLogger) Log(event Event) error { Str("ip", event.IP). Str("path", event.Path). Time("timestamp", event.Timestamp). - Str("details", event.Details). + Bool("details_present", detailsPresent). + Int("details_len", detailsLen). + Str("details_sha256", detailsDigest). Logger() if event.Success { @@ -151,6 +156,14 @@ func (c *ConsoleLogger) Log(event Event) error { return nil } +func summarizeAuditDetails(details string) (present bool, length int, digest string) { + if details == "" { + return false, 0, "" + } + sum := sha256.Sum256([]byte(details)) + return true, len(details), hex.EncodeToString(sum[:]) +} + // Query returns an empty slice for the console logger. // Console logs are not queryable - use enterprise version for persistent storage. func (c *ConsoleLogger) Query(filter QueryFilter) ([]Event, error) { diff --git a/pkg/audit/audit_test.go b/pkg/audit/audit_test.go index a90bc04d8..49f4b047c 100644 --- a/pkg/audit/audit_test.go +++ b/pkg/audit/audit_test.go @@ -5,6 +5,24 @@ import ( "time" ) +func TestSummarizeAuditDetails(t *testing.T) { + present, length, digest := summarizeAuditDetails("apiToken=secret-value") + if !present { + t.Fatal("expected details to be marked present") + } + if length != len("apiToken=secret-value") { + t.Fatalf("length = %d", length) + } + if digest == "" { + t.Fatal("expected non-empty digest") + } + + emptyPresent, emptyLength, emptyDigest := summarizeAuditDetails("") + if emptyPresent || emptyLength != 0 || emptyDigest != "" { + t.Fatalf("unexpected empty summary: %v %d %q", emptyPresent, emptyLength, emptyDigest) + } +} + func TestConsoleLogger_Log(t *testing.T) { logger := NewConsoleLogger() diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 11d45c81c..02f3e8673 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -32,7 +32,11 @@ func (f *FlexInt) UnmarshalJSON(data []byte) error { // Try as float (handles cpulimit like 1.5) var fl float64 if err := json.Unmarshal(data, &fl); err == nil { - *f = FlexInt(int(fl)) + intVal, ok := intFromFloat64TruncChecked(fl) + if !ok { + return fmt.Errorf("float value out of range for FlexInt") + } + *f = FlexInt(intVal) return nil } @@ -49,7 +53,11 @@ func (f *FlexInt) UnmarshalJSON(data []byte) error { } // Convert to int - *f = FlexInt(int(floatVal)) + intVal, ok := intFromFloat64TruncChecked(floatVal) + if !ok { + return fmt.Errorf("float value out of range for FlexInt") + } + *f = FlexInt(intVal) return nil } @@ -1581,8 +1589,8 @@ func (a *VMIpAddress) UnmarshalJSON(data []byte) error { if err != nil { prefix = 0 } - if prefix > math.MaxInt { - prefix = math.MaxInt + if prefix > 128 { + prefix = 128 } a.Prefix = int(prefix) return nil diff --git a/pkg/proxmox/client_test.go b/pkg/proxmox/client_test.go index 65b1e5efc..8ba8e5500 100644 --- a/pkg/proxmox/client_test.go +++ b/pkg/proxmox/client_test.go @@ -581,6 +581,16 @@ func TestFlexIntUnmarshalJSON(t *testing.T) { input: "1000000", want: FlexInt(1000000), }, + { + name: "float overflow", + input: "1e309", + wantErr: true, + }, + { + name: "string float overflow", + input: `"1e309"`, + wantErr: true, + }, { name: "invalid string", input: `"not a number"`, @@ -618,6 +628,19 @@ func TestFlexIntUnmarshalJSON(t *testing.T) { } } +func TestVMIpAddressUnmarshalJSONCapsPrefix(t *testing.T) { + var addr VMIpAddress + if err := addr.UnmarshalJSON([]byte(`{"ip-address":"2001:db8::1","prefix":"18446744073709551615"}`)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if addr.Address != "2001:db8::1" { + t.Fatalf("Address = %q", addr.Address) + } + if addr.Prefix != 128 { + t.Fatalf("Prefix = %d, want 128", addr.Prefix) + } +} + func TestParseUint64Flexible(t *testing.T) { tests := []struct { name string diff --git a/pkg/proxmox/intconv.go b/pkg/proxmox/intconv.go new file mode 100644 index 000000000..b09628da3 --- /dev/null +++ b/pkg/proxmox/intconv.go @@ -0,0 +1,44 @@ +package proxmox + +import "math" + +const ( + maxInt = int(^uint(0) >> 1) + minInt = -maxInt - 1 +) + +func intFromInt64Checked(v int64) (int, bool) { + if v > int64(maxInt) || v < int64(minInt) { + return 0, false + } + return int(v), true +} + +func intFromUint64Checked(v uint64) (int, bool) { + if v > uint64(maxInt) { + return 0, false + } + return int(v), true +} + +func intFromFloat64RoundedChecked(v float64) (int, bool) { + if math.IsNaN(v) || math.IsInf(v, 0) { + return 0, false + } + rounded := math.Round(v) + if rounded > float64(maxInt) || rounded < float64(minInt) { + return 0, false + } + return int(rounded), true +} + +func intFromFloat64TruncChecked(v float64) (int, bool) { + if math.IsNaN(v) || math.IsInf(v, 0) { + return 0, false + } + truncated := math.Trunc(v) + if truncated > float64(maxInt) || truncated < float64(minInt) { + return 0, false + } + return int(truncated), true +} diff --git a/pkg/proxmox/replication.go b/pkg/proxmox/replication.go index 9bb4fa2e1..0be466874 100644 --- a/pkg/proxmox/replication.go +++ b/pkg/proxmox/replication.go @@ -369,33 +369,31 @@ func intFromAny(value interface{}) (int, bool) { case int32: return int(v), true case int64: - return int(v), true + return intFromInt64Checked(v) case uint: - return int(v), true + return intFromUint64Checked(uint64(v)) case uint8: return int(v), true case uint16: return int(v), true case uint32: - return int(v), true + return intFromUint64Checked(uint64(v)) case uint64: - return int(v), true + return intFromUint64Checked(v) case float32: - if math.IsNaN(float64(v)) || math.IsInf(float64(v), 0) { - return 0, false - } - return int(math.Round(float64(v))), true + return intFromFloat64RoundedChecked(float64(v)) case float64: - if math.IsNaN(v) || math.IsInf(v, 0) { + return intFromFloat64RoundedChecked(v) + case json.Number: + s := strings.TrimSpace(v.String()) + if i, err := v.Int64(); err == nil { + return intFromInt64Checked(i) + } + if !looksLikeFloatLiteral(s) { return 0, false } - return int(math.Round(v)), true - case json.Number: - if i, err := v.Int64(); err == nil { - return int(i), true - } - if f, err := v.Float64(); err == nil && !math.IsNaN(f) && !math.IsInf(f, 0) { - return int(math.Round(f)), true + if f, err := v.Float64(); err == nil { + return intFromFloat64RoundedChecked(f) } case string: s := strings.TrimSpace(v) @@ -403,15 +401,22 @@ func intFromAny(value interface{}) (int, bool) { return 0, false } if i, err := strconv.ParseInt(s, 10, 64); err == nil { - return int(i), true + return intFromInt64Checked(i) } - if f, err := strconv.ParseFloat(s, 64); err == nil && !math.IsNaN(f) && !math.IsInf(f, 0) { - return int(math.Round(f)), true + if !looksLikeFloatLiteral(s) { + return 0, false + } + if f, err := strconv.ParseFloat(s, 64); err == nil { + return intFromFloat64RoundedChecked(f) } } return 0, false } +func looksLikeFloatLiteral(s string) bool { + return strings.ContainsAny(s, ".eE") +} + func boolFromAny(value interface{}) (bool, bool) { switch v := value.(type) { case nil: diff --git a/pkg/proxmox/replication_test.go b/pkg/proxmox/replication_test.go index ec46f50c7..d48360200 100644 --- a/pkg/proxmox/replication_test.go +++ b/pkg/proxmox/replication_test.go @@ -112,14 +112,19 @@ func TestIntFromAny(t *testing.T) { {"json.Number int", json.Number("42"), 42, true}, {"json.Number float", json.Number("42.6"), 43, true}, {"json.Number invalid", json.Number("abc"), 0, false}, + {"json.Number int overflow", json.Number("9223372036854775808"), 0, false}, // string {"string int", "42", 42, true}, {"string negative", "-42", -42, true}, {"string float", "42.6", 43, true}, + {"string int overflow", "9223372036854775808", 0, false}, + {"string float overflow", "1e309", 0, false}, {"string empty", "", 0, false}, {"string whitespace", " 42 ", 42, true}, {"string invalid", "abc", 0, false}, + {"uint64 overflow", uint64(maxInt) + 1, 0, false}, + {"float64 overflow", float64(maxInt) * 2, 0, false}, } for _, tc := range tests {