OpenPGP: Mitigate some RuntimeExceptions uncovered using Fuzzing #2123
Conversation
…9/Ed448 signatures
This is a dedicated exception type that is thrown for malformed OpenPGP packets
…algorithm as MalformedPacketException
…edPacketException for malformed packets with unexpected kdf parameter length
…known AEAD algorithms as IOException
… subpacket encoding length to max 5mb
…alformedPacketExceptions
…ng subpacket parsing as MalformedPacketException
…rmed fingerprint lengths as MalformedPacketException
…malformed packets with negative IV
…known AEAD algorithm as MalformedPacketException
…nction in ECDHPublicBCPGKey) as MalformedPacketException
…dDetectionCodePacket
|
Hi @vanitasvitae, Regarding the UserAttributeSubpacketInputStream.MAX_LEN setting (currently 5MB):
|
|
Personally I think 2^30 is still unreasonable (or at least unrealistic :D), but its still an improvement over the current 2^64 :D Is it okay, if I leave it as a |
|
While 2³⁰ bytes (~1GB) still feels excessive for real-world use cases, it’s a practical upper bound and certainly more sane than the 2⁶⁴ max. I updated the logic in UserAttributeSubpacketInputStream.readPacket() to check StreamUtil.flag_eof before evaluating bodyLen, since a true flag_eof may validly result in a bodyLen of -1. This avoids potential false positives when handling EOF-flagged packets. Thank you. |
|
Please do not add mutable statics. If some limit is warranted here, use org.bouncycastle.util.Properties class for configuration. However limit checking should begin at the top/outermost level with a constraint on the total size of the input. Low-level classes could be optimized to spot that their length is greater than the remaining limit. It looks like SignatureSubpacketInputStream started to head in that direction. |
…natureSubpacketInputStream
vanitasvitae
force-pushed
the
fuzzHardening
branch
from
71213cf to
07d9e84
Compare
…packet as MalformedPacketException
v3 and earlier OpenPGP keys can only use RSA as key algorithm. This commit changes the behavior of BC to throw a PGPException instead of a ClassCastException when encountering a malformed key.
…ESK/SEIPD/OED packet combinations and rethrow as MalformedPacketException
vanitasvitae
force-pushed
the
fuzzHardening
branch
2 times, most recently
from
83f35f7 to
ade1c0c
Compare
There was a problem hiding this comment.
Is it possible to catch and rethrow inside PGPEncryptedDataList? It would be "less surprising" amongst other things if that could be done.
Also, I've left PGPTrust as it is, wasn't quite sure what was going on. Have added the others.
There was a problem hiding this comment.
bobyLen checks should be after flag checks, have moved.
* 1.82: (1360 commits) partial addition of Java25 support removed overlap check (unsafe) minor compatibility fixes. minor compatibility fixes. updated to 1.82. Pqc hqc update v5 removed unnecessary public "internal" method. updated from PR list updated with current 1.82 PRs and bug fixes added PQC sigs further clean up of old algorithm names added named PREHASH algorithms - relates to github bcgit#2162 cleaned up algorithm list for composites. Added publish.gradle stub added purpose check, marked deprecated method corrected CertPath issue - relates to github bcgit#2152 set name of artifact to bc-<version>-bom fixed probable null pointer issue - relates to github bcgit#1907 merge of github bcgit#2123 with minor corrections error prone fix. added parameter setting to allow for pre-hash calculation - relates to github bcgit#2162 removed use of deprecated class ...
4 participants
Hey!
I did run junit-jazzer agains some PGPainless functions and uncovered some bugs in the bcpg codebase.
The commits in this PR harmonize the way malformed or strange packets are handled, by preventing runtime exceptions, such as IndexOutOfBounds, NegativeArraySize, IllegalArgumentExceptions and OutOfMemory errors.
I'm only just getting started with fuzzing, but I can highly recommend it!