steveloughran commented on PR #3562:
URL: https://github.com/apache/parquet-java/pull/3562#issuecomment-4938149764
...going through this, looks like claude wasn't that good, but it's
highlighted that other bits of the existing code in `getArrayInfo()` and
`getObjectInfo()` and flag the same issue.
Rather than inserting (long) wherever I think it's needed, I'm actually
putting readUnsigned() into a long wherever that value is coming from the
stream, and only casting to an int after the validation is done.
now I'm going through all the variant logic looking for uses of the output
of readUnsigned where the immediate action is some addition. And yes, it's
pretty common, such as in
`getMetadataKey()`.
```java
int dataPos = offsetListPos + (dictSize + 1) * offsetSize;
int offset = readUnsigned(metadata, offsetListPos + (id) * offsetSize,
offsetSize);
int nextOffset = readUnsigned(metadata, offsetListPos + (id + 1) *
offsetSize, offsetSize);
if (offset > nextOffset) {
throw new IllegalStateException(String.format("Invalid offset: %d.
next offset: %d", offset, nextOffset));
}
checkIndex(dataPos + nextOffset - 1, metadata.limit());
```
This then is going to be the code policy here
* all reads of data from that source are treated as malicious and
potentially overflowing
* I'll have to audit them all
* using long as the int is actually a fairly effective way of tainting them
in the codebase so they don't get used as array offsets or similar without
validation
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
