Hi,
I found that javax.crypto.Mac.doFinal(byte[], int) does not validate
negative outOffset, causing it to throw ArrayIndexOutOfBoundsException
instead of a proper exception.
The bug:
// Mac.java, line 624
if (output == null || output.length - outOffset < macLen) {
throw new ShortBufferException("Cannot store MAC in output buffer");
}
byte[] mac = doFinal();
System.arraycopy(mac, 0, output, outOffset, macLen);
When outOffset = -1 and output.length = 96:
- 96 - (-1) = 97 >= 32 → check passes
- System.arraycopy(mac, 0, output, -1, 32) → throws
ArrayIndexOutOfBoundsException
This is inconsistent with the rest of the JCE API:
- Mac.update(byte[], int, int) checks offset < 0 → IllegalArgumentException
- Cipher.doFinal(byte[], int) checks outputOffset < 0 →
IllegalArgumentException
- Signature.sign(byte[], int, int) checks offset < 0 → IllegalArgumentException
Reproducer:
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(new byte[32], "HmacSHA256"));
mac.update(new byte[]{1, 2, 3});
mac.doFinal(new byte[96], -1);
// Expected: IllegalArgumentException or ShortBufferException
// Actual: ArrayIndexOutOfBoundsException
The fix:
Separate null/negative-offset validation (IllegalArgumentException) from
buffer-too-small validation (ShortBufferException):
if (output == null || outOffset < 0) {
throw new IllegalArgumentException("Bad arguments");
}
int macLen = getMacLength();
if (output.length - outOffset < macLen) {
throw new ShortBufferException("Cannot store MAC in output buffer");
}
The patch includes a jtreg test covering: offset=-1, Integer.MIN_VALUE,
null output, small buffer, and valid offsets.
Webrev: https://github.com/wenshao/jdk/tree/fix/mac-doFinal-negative-offset
<https://github.com/wenshao/jdk/tree/fix/mac-doFinal-negative-offset >
Thanks,
Shaojin Wen
