When get_bitmap() decodes a bitmap whose end falls on a byte boundary,
it reads one byte past the position that the caller's
nf_h323_error_boundary() check validated.
For a bitmap of b bits starting at bit offset bs->bit, let
l = bs->bit + b. The caller guards the call with
nf_h323_error_boundary(bs, 0, b), which reserves ceil(l / 8) bytes.
In the (l < 32) path get_bitmap() first consumes l >> 3 whole bytes in
the loop and then unconditionally reads one more byte:
if (l < 32) {
v |= (unsigned int)(*bs->cur) << shift;
...
When l is a multiple of 8 (l == 16 or l == 24), l >> 3 already equals
ceil(l / 8), so the loop has consumed every byte the boundary check
reserved and the extra read touches *bs->end -- one byte out of bounds.
The value read is shifted below the final mask (0xffffffff << (32 - b))
and discarded, so this is a pure over-read, but it still crosses the
boundary nf_h323_error_boundary() is meant to enforce.
The extra byte is only needed when the bitmap spills partial bits into
a further byte, i.e. when (l & 7) is non-zero; for a byte-aligned end
the loop has already read everything. Guard the read accordingly.
Both get_bitmap() call sites in decode_seq() (the field bitmap and the
extension bitmap) are affected, and the decoder runs on
attacker-supplied H.323/H.245/RAS payloads when the nf_conntrack_h323
helper is active.
Reachability and behavioural equivalence were verified with a userspace
libFuzzer/ASan harness driving the unmodified decoder against
exactly-sized buffers; the fix removes the over-read while producing
byte-identical decode output across the whole corpus.
This was found by incomplete-fix analysis -- reviewing the recent
out-of-bounds read fixes in this decoder (decode_choice(), decode_int(),
DecodeQ931()) for sibling sites of the same class. The code review and
harness work were LLM-assisted.
Fixes: ec8a8f3c31dd ("netfilter: nf_ct_h323: Extend nf_h323_error_boundary to work on bits as well")
Signed-off-by: Duc Anh Luu <redacted>
---
net/netfilter/nf_conntrack_h323_asn1.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 6830c9da3507..8c04925b7007 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -229,7 +229,8 @@ static unsigned int get_bitmap(struct bitstr *bs, unsigned int b)
v |= (unsigned int)(*bs->cur++) << shift;
if (l < 32) {
- v |= (unsigned int)(*bs->cur) << shift;
+ if (l & 7)
+ v |= (unsigned int)(*bs->cur) << shift;
v <<= bs->bit;
} else if (l > 32) {
v <<= bs->bit;--
2.43.0