Permalink
Browse files

Cleanup file length conversion to avoid overflow

In the long term this MAC is superfluous, so we should just
remove it.

// FREEBIE
  • Loading branch information...
moxie0 committed Sep 12, 2016
1 parent d40d048 commit 8df830d052c092dd246ffb41aaff422dea7bfb50
@@ -16,6 +16,7 @@
import java.io.IOException;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
@@ -174,7 +175,7 @@ private int readIncremental(byte[] buffer, int offset, int length) throws IOExce
private void verifyMac(File file, Mac mac) throws FileNotFoundException, InvalidMacException {
try {
FileInputStream fin = new FileInputStream(file);
int remainingData = (int) file.length() - mac.getMacLength();
int remainingData = Util.toIntExact(file.length()) - mac.getMacLength();
byte[] buffer = new byte[4096];
while (remainingData > 0) {
@@ -187,10 +188,10 @@ private void verifyMac(File file, Mac mac) throws FileNotFoundException, Invalid
byte[] theirMac = new byte[mac.getMacLength()];
Util.readFully(fin, theirMac);
if (!Arrays.equals(ourMac, theirMac)) {
if (!MessageDigest.isEqual(ourMac, theirMac)) {
throw new InvalidMacException("MAC doesn't match!");
}
} catch (IOException e1) {
} catch (IOException | ArithmeticException e1) {
throw new InvalidMacException(e1);
}
}
@@ -113,4 +113,12 @@ public static void wait(Object lock, long millis) {
throw new AssertionError(e);
}
}
public static int toIntExact(long value) {
if ((int)value != value) {
throw new ArithmeticException("integer overflow");
}
return (int)value;
}
}

2 comments on commit 8df830d

@jotpe

This comment has been minimized.

Show comment
Hide comment
@jotpe

jotpe Sep 17, 2016

Why did you replace Arrays.equals(byte[],byte[]) with MessageDiggest.isEqual(byte[],byte[]) ?

jotpe replied Sep 17, 2016

Why did you replace Arrays.equals(byte[],byte[]) with MessageDiggest.isEqual(byte[],byte[]) ?

@infinity0

This comment has been minimized.

Show comment
Hide comment
@infinity0

infinity0 Sep 17, 2016

@jotpe probably so it's constant time

infinity0 replied Sep 17, 2016

@jotpe probably so it's constant time

Please sign in to comment.