From ab1a2d468c4b2ac09a0ac651563653f36a73215f Mon Sep 17 00:00:00 2001 From: William Roberts Date: Fri, 15 Dec 2017 11:43:42 -0800 Subject: [PATCH] tpm2_nvwrite: fix buffer overflow As reported by clang asan: ================================================================= ==435==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe036c0902 at pc 0x0000004a326d bp 0x7ffe036c02f0 sp 0x7ffe036bfaa0 WRITE of size 2048 at 0x7ffe036c0902 thread T0 #0 0x4a326c in __asan_memcpy (/home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite+0x4a326c) #1 0x4e887b in nv_write /home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite.c:129:9 #2 0x4e82c4 in tpm2_tool_onrun /home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite.c:316:11 #3 0x4e90d9 in main /home/wcrobert/workspace/tpm2-tools/tools/tpm2_tool.c:150:11 #4 0x7fdfc968d82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #5 0x4195c8 in _start (/home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite+0x4195c8) Address 0x7ffe036c0902 is located in stack of thread T0 at offset 1538 in frame #0 0x4e846f in nv_write /home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite.c:76 This frame has 8 object(s): [32, 172) 'session_data_out' [240, 256) 'sessions_data' [272, 288) 'sessions_data_out' [304, 312) 'session_data_array' [336, 344) 'session_data_out_array' [368, 452) 'nv_public' [496, 500) 'max_data_size' [512, 1538) 'nv_write_data' <== Memory access at offset 1538 overflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/wcrobert/workspace/tpm2-tools/tools/tpm2_nvwrite+0x4a326c) in __asan_memcpy Shadow bytes around the buggy address: 0x1000406d00d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d00e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d00f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d0100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d0110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x1000406d0120:[02]f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 0x1000406d0130: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d0140: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 0x1000406d0150: 00 00 00 00 00 00 00 00 02 f2 f2 f2 f2 f2 00 f3 0x1000406d0160: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 0x1000406d0170: 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2 01 f2 00 f3 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==435==ABORTING Fix by using the max buffer's size field, not the loaded from file buffers size field. Signed-off-by: William Roberts --- tools/tpm2_nvwrite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tpm2_nvwrite.c b/tools/tpm2_nvwrite.c index 63c892f..a412898 100644 --- a/tools/tpm2_nvwrite.c +++ b/tools/tpm2_nvwrite.c @@ -127,7 +127,7 @@ static bool nv_write(TSS2_SYS_CONTEXT *sapi_context) { LOG_INFO("The data(size=%d) to be written:", nv_write_data.t.size); memcpy(nv_write_data.t.buffer, &ctx.nv_buffer.t.buffer[data_offset], - ctx.nv_buffer.t.size); + nv_write_data.t.size); TPM_RC rval = TSS2_RETRY_EXP(Tss2_Sys_NV_Write(sapi_context, ctx.auth_handle, ctx.nv_index, &sessions_data, &nv_write_data, ctx.offset + data_offset, -- 2.15.0