kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342191581
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##########
@@ -277,13 +279,14 @@ public void close() throws IOException {
WebPChunk readChunk() throws ImagingException, IOException {
while (sizeCount < fileSize) {
- int type = read4Bytes("Chunk Type", is, "Not a Valid WebP
File", ByteOrder.LITTLE_ENDIAN);
- int payloadSize = read4Bytes("Chunk Size", is, "Not a Valid
WebP File", ByteOrder.LITTLE_ENDIAN);
+ int type = read4Bytes("Chunk Type", is, "Not a valid WebP
file", ByteOrder.LITTLE_ENDIAN);
+ int payloadSize = read4Bytes("Chunk Size", is, "Not a valid
WebP file", ByteOrder.LITTLE_ENDIAN);
if (payloadSize < 0) {
throw new ImagingException("Chunk Payload is too long:" +
payloadSize);
}
boolean padding = (payloadSize % 2) != 0;
- int chunkSize = payloadSize + 8 + (padding ? 1 : 0);
+ int extraPadding = Math.addExact(8, (padding ? 1 : 0));
Review Comment:
>The maximum value of extraPadding is 9, so there is no risk of overflow.
`9` as per specification? Or is there a constant somewhere?
Asking as we had security issues that had to be fixed (with some urgency,
which is no fun) where users crafted images with custom metadata, many times
invalid image formats.
So we just need to confirm if `9` is the maximum value, or if the user can
somehow manipulate it.
>Furthermore, the name extraPadding is somewhat misleading, since padding
only refers to the padding at the end of the chunk.
Sorry, I just changed that, pushing a new version now... it uses a function
to avoid having to delcare variables.
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##########
@@ -61,22 +74,41 @@ public String getTypeDescription() {
(byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
}
+ /**
+ * @return the payload size.
+ */
public int getPayloadSize() {
return size;
}
+ /**
+ * @return the chunk size.
+ */
public int getChunkSize() {
// if chunk size is odd, a single padding byte is added
int padding = (size % 2) != 0 ? 1 : 0;
// Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n
bytes) + Padding
- return 4 + 4 + size + padding;
+ int chunkFourCCandSize = 4 + 4;
+ int paddedSize = Math.addExact(size, padding);
+ return Math.addExact(chunkFourCCandSize, paddedSize);
Review Comment:
Good idea. Let me try that instead.
--
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: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
