java.lang.ArrayIndexOutOfBoundsException from StreamDeserializationContext.read() if Hash.from() is called for 1048576+1 bytes

[gregoreficint]

When using the Hash.from function with more than 1048576 bytes, an java.lang.ArrayIndexOutOfBoundsException is thrown due to using an out-of-bounds index on a byte array.

The following test fails due to the above exception for the current version of java-opentimestamps:

@Test
public void hashFd3() throws Exception {
    String hash = "2CB74EDBA754A81D121C9DB6833704A8E7D417E5B13D1A19F4A52F007D644264";
    int size = 1048576+1;
    byte[] buffer = new byte[size];
    Hash myHash=Hash.from(buffer, OpSHA256._TAG);
    assertArrayEquals(Utils.hexToBytes(hash), myHash.getValue());
}

The function Hash.from uses OpCrypto.hashFd(byte[]), which in turn uses OpCrypto.hashFd(StreamDeserializationContext). The code looks like this:

public byte[] hashFd(StreamDeserializationContext ctx) throws NoSuchAlgorithmException {
    MessageDigest digest = MessageDigest.getInstance(this._HASHLIB_NAME());
    byte[] chunk = ctx.read(1048576);

    while (chunk != null && chunk.length > 0) {
        digest.update(chunk);
        chunk = ctx.read(1048576);
    }

    byte[] hash = digest.digest();

    return hash;
}

The while loop reads always blocks of 1048576 bytes. Lets assume, we have 1048577 bytes and see how the original read function behaves:

public byte[] read(int l) {
    if (this.counter == this.buffer.length) {
        return null;
    }

    if (l > this.buffer.length) {
        l = this.buffer.length;
    }

    // const uint8Array = new Uint8Array(this.buffer,this.counter,l);
    byte[] uint8Array = Arrays.copyOfRange(this.buffer, this.counter, this.counter + l);
    this.counter += l;

    return uint8Array;
}

Let's look at two iterations of hashFd from the perspective of read

1. Iteration Is 1048576>1048577? -> No this.counter=0+1048576 =1048576 uint8Array = Arrays.copyOfRange(this.buffer, 0, 0+1048576);
2. Iteration Is 1048576>1048577? -> No this.counter=1048576 +1048576 =2097152 uint8Array = Arrays.copyOfRange(this.buffer, 1048576, 2097152); Obviously, 2097152 is out of bounds since we only have 1048577 bytes.

The problem is that the if-statement does not take into account what has been read in the previous iterations, thus the previous if-statement works only till 1048576 bytes since this only requires one iteration:

public byte[] read(int l) {
  if (this.counter == this.buffer.length) {
      return null;
  }

  if (l+this.counter > this.buffer.length) {
      l = this.buffer.length-this.counter;
  }

  // const uint8Array = new Uint8Array(this.buffer,this.counter,l);
  byte[] uint8Array = Arrays.copyOfRange(this.buffer, this.counter, this.counter + l);
  this.counter += l;

  return uint8Array;
}

If we now make the two iterations, we get the right result:

1. Iteration Is 1048576+0>1048577? -> No this.counter=0+1048576 =1048576 uint8Array = Arrays.copyOfRange(this.buffer, 0, 0+1048576);
2. Iteration Is 1048576+1048576>1048577 -> Yes, set l=1048577-1048576=1 this.counter=1048576 +1 =1048577 uint8Array = Arrays.copyOfRange(this.buffer, 1048576, 1048577);

Now, we have the right bounds.

Fix inkl. tests is available in pull request https://github.com/opentimestamps/java-opentimestamps/pull/54#issue-552337322.

[gregoreficint]

Fixed and merged now, so I close the issue