GHSA-7c37-gx6w-8vc5

Описание

Summary

CertVerifier.Verify() in pkg/git/verifier.go unconditionally dereferences certs[0] after sd.GetCertificates() without checking the slice length. A CMS/PKCS7 signed message with an empty certificate set is a structurally valid DER payload; GetCertificates() returns an empty slice with no error, causing an immediate index-out-of-range panic. On the gitsign --verify code path (the GPG-compatible mode invoked by git verify-commit), the panic is silently recovered by internal/io/streams.go's Wrap() function, which returns nil instead of an error. main.go then exits with code 0, causing exit-code-only verification callers to interpret the failed verification as success.

Severity

Medium (CVSS 3.1: 5.8)

CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:L/A:L

• Attack Vector: Network — attacker pushes a commit carrying a crafted signature to any accessible repository, or delivers the signature file out-of-band
• Attack Complexity: Low — stripping certificates from a PKCS7 object requires only standard ASN.1 tooling
• Privileges Required: None — writing to an accessible repo (or creating a repo a victim clones) is sufficient
• User Interaction: Required — victim must run git verify-commit, gitsign --verify, or an equivalent verification step
• Scope: Unchanged
• Confidentiality Impact: None
• Integrity Impact: Low — exit-code-only callers (scripts, some CI pipelines) treat the panicked verification as success; git's own status-fd path checks for GOODSIG and is therefore partially protected
• Availability Impact: Low — the verification process aborts via panic on every invocation with such a signature

Affected Component

• pkg/git/verifier.go — (*CertVerifier).Verify (line 114)
• internal/io/streams.go — (*Streams).Wrap (lines 71–84, the recovery that returns nil on panic)

CWE

• CWE-129: Improper Validation of Array Index
• CWE-390: Detection of Error Condition Without Action Taken (panic swallowed, nil returned)

Description

Unconditional index dereference after GetCertificates

CertVerifier.Verify() parses the incoming signature as CMS/PKCS7 and calls GetCertificates() to extract the signer's certificate before any signature math takes place:

GetCertificates() delegates to sd.psd.X509Certificates() (the upstream smimesign/ietf-cms library). RFC 5652 §5.1 marks the certificates field in SignedData as OPTIONAL, and an empty or absent set is a structurally valid CMS message. The library returns (nil, nil) or ([]*, nil) for such a message — an empty slice with no error — so the length check on err is irrelevant:

There is no length guard anywhere between GetCertificates() and the certs[0] dereference.

Panic recovery silently returns exit 0

All root-command invocations (including gitsign --verify, which git calls for verify-commit) are wrapped by (*Streams).Wrap:

Wrap uses a bare recover() inside a defer:

In Go, a recover() in a defer does not modify the enclosing function's return value unless named returns are used. When fn() panics, the defer fires, prints the panic message and stack trace to TTYOut, and then Wrap returns the zero value for error — which is nil.

main.go then sees nil from rootCmd.Execute() and exits 0:

GPG status-fd provides partial protection for git verify-commit

git verify-commit passes --status-fd=1 to gitsign. The GPG status protocol requires GOODSIG in the status output for git to treat the signature as valid. In commandVerify, EmitGoodSig is only called after v.Verify() succeeds:

Because the panic fires inside v.Verify(), only NEWSIG (not GOODSIG) is written to the status-fd. Modern git reads this output and still considers the commit unverified. However, scripts and CI tools that check only the exit code of gitsign --verify see exit 0 and consider verification successful.

Execution chain to impact

1. Attacker strips all certificates from a valid gitsign PKCS7 signature using sd.SetCertificates([]*x509.Certificate{}) and re-serializes the message.
2. Attacker attaches this certificate-free signature as the gpgsig field of a commit and pushes it to an accessible repository (or delivers the .pem file directly).
3. Victim runs gitsign --verify <sig> <data> or git verify-commit <commit> (which internally invokes gitsign --verify).
4. CertVerifier.Verify() panics at certs[0] with index out of range [0] with length 0.
5. Wrap() recovers the panic and returns nil; process exits 0.
6. Any caller that checks only the exit code considers verification successful.

Proof of Concept

Expected output after go run main.go --verify /tmp/gitsign-badsig.pem /tmp/gitsign-data.bin; echo "exit: $?":

Impact

• Authentication bypass for exit-code callers: Any script or CI pipeline running gitsign --verify and checking only $? will treat the panicked verification as a success (exit 0). This allows an attacker to make a commit appear verified without a valid signature.
• Denial of service: Every verification attempt against a crafted signature panics, preventing legitimate verification output from being produced.
• Misleading output: The panic stack trace is written to TTYOut (stderr in non-TTY environments), which may be silently discarded by callers that redirect stderr.
• Partial bypass of git verify-commit: git itself is protected by the GOODSIG check on the status-fd; however, the exit-code bypass affects auxiliary tooling that wraps gitsign --verify directly.

Recommended Remediation

Option 1: Guard the slice access (preferred — lowest layer, protects all callers)

Add an explicit length check in CertVerifier.Verify() immediately after GetCertificates():

This produces a clean error at the source instead of a panic, propagated through commandVerify as a non-nil return, so Wrap returns it, Execute() returns it, and main.go exits 1.

Option 2: Return an error instead of nil on panic recovery

Fix Wrap() to return an error when it recovers a panic, so that all callers reliably see a non-zero exit code:

This is a defense-in-depth fix. It ensures that any future panic in a command results in exit 1 rather than 0. Option 1 should be applied regardless; Option 2 prevents similar bypass bugs from any other panic source.

Credit

This vulnerability was discovered and reported by bugbunny.ai.