From 03fc0c626d9fa91219442bec274db432d8f413f6 Mon Sep 17 00:00:00 2001 From: Nehal J Wani Date: Thu, 13 Mar 2014 03:14:11 +0530 Subject: [PATCH] vircrypto: fix Invalid write in virCryptoHashString() While running vircryptotest, it was found that valgrind pointed out the following error: ==27453== Invalid write of size 1 ==27453== at 0x4C7D7C9: virCryptoHashString (vircrypto.c:76) ==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41) ==27453== by 0x402A11: virtTestRun (testutils.c:199) ==27453== by 0x401AD5: mymain (vircryptotest.c:76) ==27453== by 0x40318D: virtTestMain (testutils.c:782) ==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ==27453== Address 0x51f0541 is 0 bytes after a block of size 65 alloc'd ==27453== at 0x4A0577B: calloc (vg_replace_malloc.c:593) ==27453== by 0x4C69F2E: virAllocN (viralloc.c:189) ==27453== by 0x4C7D76B: virCryptoHashString (vircrypto.c:69) ==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41) ==27453== by 0x402A11: virtTestRun (testutils.c:199) ==27453== by 0x401AD5: mymain (vircryptotest.c:76) ==27453== by 0x40318D: virtTestMain (testutils.c:782) ==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ==27453== ...and many more. Two observations: hashstrlen was already set to include the trailing NUL byte (so writing to hashstrlen as the array offset was indeed writing one byte beyond bounds), and VIR_ALLOC_N already guarantees zero-initialization (so we already have a trailing NUL without needing to explicitly write one). Signed-off-by: Eric Blake --- src/util/vircrypto.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 3af3aa3bb0..39a479af48 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -73,7 +73,6 @@ virCryptoHashString(virCryptoHash hash, (*output)[i * 2] = hex[(buf[i] >> 4) & 0xf]; (*output)[(i * 2) + 1] = hex[buf[i] & 0xf]; } - (*output)[hashstrlen] = '\0'; return 0; }