`AHasher::write` for the fallback algorithm and AES implementations discards up to 15 trailing bytes

[yorickpeterse]

While working on porting aHash to https://github.com/inko-lang/inko I ran into the following:

aHash/src/fallback_hash.rs

Lines 170 to 192 in a9d649d

	fn write(&mut self, input: &[u8]) {
	let mut data = input;
	let length = data.len() as u64;
	//Needs to be an add rather than an xor because otherwise it could be canceled with carefully formed input.
	self.buffer = self.buffer.wrapping_add(length).wrapping_mul(MULTIPLE);
	//A 'binary search' on sizes reduces the number of comparisons.
	if data.len() > 8 {
	if data.len() > 16 {
	let tail = data.read_last_u128();
	self.large_update(tail);
	while data.len() > 16 {
	let (block, rest) = data.read_u128();
	self.large_update(block);
	data = rest;
	}
	} else {
	self.large_update([data.read_u64().0, data.read_last_u64()].convert());
	}
	} else {
	let value = read_small(data);
	self.large_update(value.convert());
	}
	}

Specifically the following part appears problematic, at least at first sight:

aHash/src/fallback_hash.rs

Lines 180 to 184 in a9d649d

	while data.len() > 16 {
	let (block, rest) = data.read_u128();
	self.large_update(block);
	data = rest;
	}

Imagine the input buffer is 18 bytes long. The following condition is then true:

aHash/src/fallback_hash.rs

Line 180 in a9d649d

while data.len() > 16 {

Which means it runs this:

aHash/src/fallback_hash.rs

Lines 181 to 183 in a9d649d

	let (block, rest) = data.read_u128();
	self.large_update(block);
	data = rest;

The read_u128 will then return (16 bytes of data, 2 bytes of data). Because of the data = rest assignment, the next hit of while data.len() > 16 translates into 2 > 16 and thus the loop isn't run.

Or more precisely, if the input size is not a multiple of 16 then up to 15 trailing bytes of input are not hashed.

I have a hard time imagining that this is in fact desired. Should that entire method not run in a loop to account for those trailing 15 bytes, or did I perhaps overlook something that causes those trailing bytes to still be hashed?

[yorickpeterse]

The AES version has a similar loop and (unless I'm horribly wrong/stupid here) would suffer from the same flaw:

aHash/src/aes_hash.rs

Lines 174 to 185 in a9d649d

	while data.len() > 64 {
	let (blocks, rest) = data.read_u128x4();
	current[0] = aesdec(current[0], blocks[0]);
	current[1] = aesdec(current[1], blocks[1]);
	current[2] = aesdec(current[2], blocks[2]);
	current[3] = aesdec(current[3], blocks[3]);
	sum[0] = shuffle_and_add(sum[0], blocks[0]);
	sum[1] = shuffle_and_add(sum[1], blocks[1]);
	sum[0] = shuffle_and_add(sum[0], blocks[2]);
	sum[1] = shuffle_and_add(sum[1], blocks[3]);
	data = rest;
	}

[yorickpeterse]

Worth adding: the write methods do encode the input size into the hash state and so you won't get identical hashes, but you are still discarding N bytes by the looks of it which would degrade the hash quality.

To illustrate, for the input aaaaaaaaaaaaaaaammmm the data variable is first set to this:

[97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 97, 109, 109, 109, 109]

After the single iteration it's set to this:

[109, 109, 109, 109]

This remainder in turn is never used in the write method.

[yorickpeterse]

Oh, I now see the following:

aHash/src/fallback_hash.rs

Line 178 in a9d649d

let tail = data.read_last_u128();

That should cover the trailing data, so I guess (lucky enough) I was just an idiot. Oops!