diff --git a/internal/audit/chain.go b/internal/audit/chain.go index faf98119..3dd1cc13 100644 --- a/internal/audit/chain.go +++ b/internal/audit/chain.go @@ -132,8 +132,17 @@ func (l *Log) VerifyChainAgainst(anchor *CheckpointAnchor) (*VerifyResult, error timestamp, tool, level string input, output, risk, sessionID sql.NullString durationMs sql.NullInt64 - success bool - storedHash sql.NullString + // success is scanned as NullInt64, not a bare bool: legitimate rows + // always store 0/1, but success and entry_hash are independent + // columns an attacker with direct DB write can set separately. A + // bare-bool scan of a poisoned NULL or out-of-range value (e.g. 2) + // fails the scan and aborts the entire verification walk with an + // opaque type error — turning a tamper into a DoS on the integrity + // check rather than the designed "chain broken at row N" verdict. + // Scanning defensively lets any non-1 value flow into the hash + // recompute below, where it correctly fails the comparison. + success sql.NullInt64 + storedHash sql.NullString ) if err := rows.Scan(&id, ×tamp, &tool, &input, &output, &risk, &level, &sessionID, &durationMs, &success, &storedHash); err != nil { @@ -151,7 +160,7 @@ func (l *Log) VerifyChainAgainst(anchor *CheckpointAnchor) (*VerifyResult, error res.HashedRows++ want := chainHash(prevHash, timestamp, tool, input.String, output.String, - risk.String, level, sessionID.String, durationMs.Int64, success) + risk.String, level, sessionID.String, durationMs.Int64, success.Int64 == 1) if want != storedHash.String { if res.Valid { // record only the first break res.Valid = false diff --git a/internal/audit/chain_test.go b/internal/audit/chain_test.go index faa60aef..629b4830 100644 --- a/internal/audit/chain_test.go +++ b/internal/audit/chain_test.go @@ -199,3 +199,39 @@ func TestVerifyChain_ContinuesAcrossReopen(t *testing.T) { t.Errorf("chain should span the reopen: %+v", res) } } + +// TestVerifyChain_PoisonedSuccessReportsBreak pins that a row whose success +// column is poisoned out-of-range (2) or to NULL by an attacker with direct DB +// write is reported as a chain break — not surfaced as an opaque scan error +// that aborts the entire verification walk. success and entry_hash are +// independent columns; scanning success as a bare bool turned a tamper into a +// DoS on the forensic integrity check. +func TestVerifyChain_PoisonedSuccessReportsBreak(t *testing.T) { + for _, tc := range []struct { + name string + poke string + }{ + {"out_of_range", `UPDATE audit_log SET success=2 WHERE id=2`}, + {"null", `UPDATE audit_log SET success=NULL WHERE id=2`}, + } { + t.Run(tc.name, func(t *testing.T) { + l := openTestLog(t) + for i := 0; i < 4; i++ { + l.Record("nfc", map[string]int{"i": i}, "ok", "low", LevelAction, 0, true) + } + if _, err := l.db.Exec(tc.poke); err != nil { + t.Fatalf("poison: %v", err) + } + res, err := l.VerifyChain() + if err != nil { + t.Fatalf("VerifyChain errored on a poisoned success value (must report a break, not abort the walk): %v", err) + } + if res.Valid { + t.Fatal("poisoned success value not detected as a chain break") + } + if res.FirstBrokenID != 2 { + t.Errorf("FirstBrokenID = %d, want 2", res.FirstBrokenID) + } + }) + } +}