internal/transparency at 459add6e04, reviewed by codex, verified by claude-code.
VerifyInclusion accepts an empty JSON proof object and passes its nil RootHash to Rekor's converter, which panics instead of rejecting the malformed entry.
#!/bin/sh
set -eu
probe=internal/transparency/cold_review_panic_test.go
[ ! -e "$probe" ] || exit 1
trap 'rm -f "$probe"' EXIT HUP INT TERM
cat > "$probe" <<'EOF'
package transparency
import (
"fmt"
"runtime"
"strings"
"testing"
)
func TestColdReviewMalformedProofPanics(t *testing.T) {
defer func() {
p := recover()
if p == nil {
t.Fatal("defect not demonstrated: verifier returned without panic")
}
buf := make([]byte, 16384)
stack := string(buf[:runtime.Stack(buf, false)])
if !strings.Contains(stack, "tle.GenerateTransparencyLogEntry") {
t.Fatalf("unexpected panic location: %v\n%s", p, stack)
}
fmt.Printf("DEFECT: proof {} caused panic instead of rejection: %v\n%s", p, stack)
}()
w := &rekorWitness{}
ok, detail := w.VerifyInclusion(Entry{LogID: "00", InclusionProof: []byte(`{}`)}, []byte(`{}`))
t.Fatalf("defect not demonstrated: returned ok=%v detail=%q", ok, detail)
}
EOF
timeout -k 1s 55s env GOPROXY=off GOSUMDB=off CORRALAI_REKOR_INTEGRATION=0 go test ./internal/transparency -run '^TestColdReviewMalformedProofPanics$' -count=1 -timeout=40s -v 2>&1=== RUN TestColdReviewMalformedProofPanics
DEFECT: proof {} caused panic instead of rejection: runtime error: invalid memory address or nil pointer dereference
goroutine 8 [running]:
github.com/pdbethke/corralai/internal/transparency.TestColdReviewMalformedProofPanics.func1()
/tmp/corral-review-3997205249/tree/internal/transparency/cold_review_panic_test.go:17 +0x99
panic({0xcbe480?, 0x15a01d0?})
/home/pdbethke/sdk/go1.26.7/src/runtime/panic.go:860 +0x13a
github.com/sigstore/rekor/pkg/tle.GenerateTransparencyLogEntry({0x0, {0xc67a20, 0x15edd60}, 0x218ff60321e8, 0x218ff5fc3dc0, 0x218ff60321e0, 0x218ff5ff4c20})
/home/pdbethke/go/pkg/mod/github.com/sigstore/rekor@v1.5.3/pkg/tle/tle.go:39 +0x10a
github.com/pdbethke/corralai/internal/transparency.(*rekorWitness).VerifyInclusion(0x218ff5e2def8, {0x0, {0xdf25c8, 0x2}, 0x0, {0x218ff6032090, 0x2, 0x2}, {0x0, 0x0, ...}, ...}, ...)
/tmp/corral-review-3997205249/tree/internal/transparency/rekor.go:249 +0x272
github.com/pdbethke/corralai/internal/transparency.TestColdReviewMalformedProofPanics(0x218ff5fead88)
/tmp/corral-review-3997205249/tree/internal/transparency/cold_review_panic_test.go:24 +0x176
testing.tRunner(0x218ff5fead88, 0xe4e320)
/home/pdbethke/sdk/go1.26.7/src/testing/testing.go:2036 +0xea
created by testing.(*T).Run in goroutine 1
/home/pdbethke/sdk/go1.26.7/src/testing/testing.go:2101 +0x4c5
--- PASS: TestColdReviewMalformedProofPanics (0.00s)
PASS
ok github.com/pdbethke/corralai/internal/transparency 0.006sI tried to refute this and could not. rekor.go:226-229 accepts any JSON that unmarshals into models.InclusionProof, so `{}` leaves RootHash, TreeSize, LogIndex and Checkpoint nil. They go into models.LogEntryAnon at rekor.go:244-247 and then to tle.GenerateTransparencyLogEntry at rekor.go:249 with no nil or Validate() check in between. The reviewer's script recovered a nil-pointer panic whose stack runs through tle.go:39 and rekor.go:249, which is the claimed location. I also looked for a recover that would turn this into a rejection. The only production caller, internal/certverify/certverify.go:207, calls w.VerifyInclusion directly, and the only recover() in the repo is in internal/mission/routing.go, which is unrelated. A malformed stored `rekor` field in a record therefore crashes `certify verify` instead of failing the rekor check.
ref e1ad3ffb51e47a22f4b851dcfc240274e5220a8002e8be561af417a04bee07d2#R1
VerifyInclusion compares only the payload hash, so a replacement envelope signed by a different key over the same payload can reuse the original envelope's inclusion proof without that replacement signature ever having been logged.
The claim is about VerifyInclusion, and the code matches it. Step 3 (rekor.go:283-293) takes only tlogEntry.GetDssePayloadHash() and compares it with sha256 of the decoded payload from envelopePayloadSHA256 (rekor.go:303-317), which reads only the `payload` field. Nothing compares the logged envelopeHash, or the logged signatures and verifier keys, against dsseEnvelope. So an envelope with the same payload but different signatures passes, even though the comment at rekor.go:281 says the check confirms the entry 'wraps THIS envelope'. I checked the one production caller as a possible mitigation. certverify.VerifyRecord (certverify.go:84) verifies rec.Signature against a pinned out-of-band Ed25519 key before it reaches the rekor check (certverify.go:199 skips it if an earlier check failed). That means an envelope signed by a different key alone fails check 1 in the current pipeline, which narrows the practical impact. It does not make the claim false: VerifyInclusion itself accepts the replacement envelope, and any other or future caller that does not check the pinned key first inherits that.
ref e1ad3ffb51e47a22f4b851dcfc240274e5220a8002e8be561af417a04bee07d2#R2
Recorded so that "found nothing here" can be told apart from "never looked here".
Finding 1 exposes a crash on malformed proof input. Finding 2 weakens the claimed binding between a logged attestation and the envelope being verified. Review covered all six scoped files and relevant callers and dependency implementations; no code was executed. The reproduction script is supplied for the external runner.
R1 stands. The reviewer's script shows the panic. `VerifyInclusion` unmarshals `{}` into a zero `models.InclusionProof` with no nil-field check and passes it to `tle.GenerateTransparencyLogEntry` at rekor.go:249, which dereferences the nil RootHash (the trace points to rekor tle.go:39). No caller recovers from the panic: I searched for recover() and the only hit is in internal/mission, and certverify.go:207 calls the function directly. R2 stands as a statement about `VerifyInclusion`. Step 3 (rekor.go:281-293) compares only the DSSE payload hash, even though its comment says it confirms the entry 'wraps THIS envelope'. Nothing compares the envelope hash or the logged signatures, so the function accepts an envelope whose signatures were never logged, as long as the payload is the same. The real-world harm is smaller than the finding implies, because the only production caller (certverify.VerifyRecord) first checks the envelope signature against an Ed25519 key it gets out-of-band (certverify.go:84), and Ed25519 is deterministic. Still, that narrows the harm, not the claim about this function, so I cannot refute it.