kali834x commented on PR #782:
URL: https://github.com/apache/commons-compress/pull/782#issuecomment-4959174828
Yes, same pattern there. The int[] overloads of parseCPSignatureReferences
and parseCPUTF8References sum the counts with a plain loop and size each
sub-array straight from counts[i], so a counts array like {-1, 2} keeps the
total non-negative, gets past the scalar guard in decodeBandInt, and then
throws NegativeArraySizeException at new CPUTF8[-1]. parseCPUTF8References even
allocates from counts[i] before decoding anything. ClassBands feeds these from
band-decoded counts (localVariableTableN, anno_N, etc.), so the same
crafted-archive path applies.
Spec-wise, count bands default to UNSIGNED5, which can't encode a negative,
but the spec allows a secondary encoding to replace the primary for any band
after the segment header, and signed codecs are eligible there. It never gives
a negative count any meaning, so treating one as corrupt data is consistent
with it.
On the maximum: the only explicit numeric limit the spec states is for the
constant pool, "the arithmetic sum of all numbers in cp_counts must be less
than 2^29" (536870912). The other count bands have no stated maximum of their
own; they're bounded indirectly by the class file format (u2, so 65535, for
interface/field/method/attribute style counts) and by each sum having to fit a
Java array. Related: sumPositive accumulates with plain +=, so enough large
counts can wrap the total back to a positive but wrong value and the failure
becomes an OOME instead of a Pack200Exception. Pack200Exception.addExact would
close that. I can put up a follow-up PR covering the two CP reference methods
and the overflow check if you want it.
Thanks for merging this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
