Skip to content

guard nil issuer key id on v2 message read path - #319

Merged
twiss merged 2 commits into
ProtonMail:mainfrom
knQzx:guard-nil-issuer-v2-read
Aug 14, 2026
Merged

twiss merged 2 commits into
ProtonMail:mainfrom
knQzx:guard-nil-issuer-v2-read

Conversation

@knQzx

@knQzx knQzx commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

A signature packet with no issuer key id causes a nil pointer dereference while reading a v2 message, so untrusted input can panic the reader. This checks the issuer key id before it is dereferenced and rejects the signature instead of panicking.

Comment thread openpgp/v2/read.go Outdated
Comment on lines 341 to 353

@twiss twiss Aug 13, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it might be clearer to move the check as close as possible to the usage.

Suggested change
invalidV3 := sc.OPSVersion == 3 && correspondingSig.Version == 6
invalidV6 := sc.OPSVersion == 6 &&
(correspondingSig.Version != 6 ||
!bytes.Equal(sc.IssuerFingerprint, correspondingSig.IssuerFingerprint) ||
!bytes.Equal(sc.Salt, correspondingSig.Salt()))
return !invalidV3 && !invalidV6 &&
sc.SigType == correspondingSig.SigType &&
sc.HashAlgorithm == correspondingSig.Hash &&
sc.PubKeyAlgo == correspondingSig.PubKeyAlgo &&
correspondingSig.IssuerKeyId != nil &&
sc.IssuerKeyId == *correspondingSig.IssuerKeyId

@knQzx
knQzx force-pushed the guard-nil-issuer-v2-read branch from b1ba59f to 6f7c01a Compare August 13, 2026 16:27
@knQzx

knQzx commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

thanks, applied. moved the nil check into the return chain right before the deref instead of the early return

@twiss
twiss force-pushed the guard-nil-issuer-v2-read branch from 365a80f to 1f99f3d Compare August 14, 2026 09:26

@twiss twiss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@twiss
twiss merged commit f7bfe56 into ProtonMail:main Aug 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants