Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions internal/audit/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, &timestamp, &tool, &input, &output, &risk, &level,
&sessionID, &durationMs, &success, &storedHash); err != nil {
Expand All @@ -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
Expand Down
36 changes: 36 additions & 0 deletions internal/audit/chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Loading