sha3: wrong results for large inputs
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Jakuje
	Sep 23 2022, 7:20 PM

Description

The SHA3 functions give wrong results for inputs larger than 4GB, because the originally size_t argument handled as unsigned int in keccak_write and leads to integer overflows. This does not happen if the data is fed into the md_write by smaller chunks. More information and reproducers are available from Clemens in the attached bug.

The fix that should solve the problem (use of the size_t) is available now at gitlab: https://gitlab.com/redhat-crypto/libgcrypt/libgcrypt-mirror/-/merge_requests/6 Comments welcomed.

I was considering updating the some of the hash tests to capture this issue, but did not find a simple way to do that yet so I will keep it on you to decide if you believe some regression test is needed here.

Details

External Link: https://bugzilla.redhat.com/show_bug.cgi?id=2129150
Version: master

Revisions and Commits

rC libgcrypt
	rC0cb29a5736cf tests/hashtest: add hugeblock & disable-hwf options and 6 gig test vectors
	rC9c828129b205 keccak: Use size_t to avoid integer overflow

Related Objects

Mentioned In: T7165: Release Libgcrypt 1.11.0
T5905: Release Libgcrypt 1.10.2

Event Timeline

Jakuje created this task.Sep 23 2022, 7:20 PM

Fix looks good to me. This could be tested with new long running test (tests/hashtest) that would allocate 4GiB+ pattern block for inputting to gcry_md_write.

Here's patch that adds 6GiB test for hashtest (with 5GiB pattern block):

0001-tests-hashtest-add-hugeblock-disable-hwf-options-and.patch17 KBDownload

neverpanic added a subscriber: neverpanic.Sep 26 2022, 1:00 PM

The test looks good. I hope I changed the API in all the hw optimized implementations.

My poor old laptop - its RAM will now have a hard time to run the huge tests ;-)

• werner triaged this task as Normal priority.Sep 26 2022, 7:36 PM

I've tested the different hw implementations (amd64, arm64, s390x) and they are all ok.

jukivili added a commit: rC9c828129b205: keccak: Use size_t to avoid integer overflow.Sep 27 2022, 7:34 AM

jukivili added a commit: rC0cb29a5736cf: tests/hashtest: add hugeblock & disable-hwf options and 6 gig test vectors.

One nit that I overlooked initially is the memory leak, which is fixed with the following patch:

libgcrypt-leak.patch732 BDownload

Patch applied to master, thanks.

One more nit regarding to the test is the format string for size_t which was using %d instead of %zu. This is fixed by the attached patch:

libgcrypt-format.patch1 KBDownload

• canti59 added a commit: rG8046fcac63db: po: Update Czech translation..Oct 14 2022, 2:10 AM

• gniibe removed a commit: rG8046fcac63db: po: Update Czech translation..Oct 14 2022, 2:50 AM

• gniibe moved this task from Backlog to Next on the FIPS board.Nov 2 2022, 9:36 AM

• gniibe moved this task from Next to Ready for release on the FIPS board.

• gniibe changed the task status from Open to Testing.Nov 7 2022, 7:14 AM

• werner mentioned this in T5905: Release Libgcrypt 1.10.2.Nov 14 2023, 12:55 PM

• werner moved this task from Backlog to For 1.10 on the libgcrypt board.Nov 14 2023, 1:14 PM

• werner closed this task as Resolved.Nov 14 2023, 1:18 PM

• werner claimed this task.

• werner mentioned this in T7165: Release Libgcrypt 1.11.0.Jun 19 2024, 11:37 AM