[llvm] r321030 - Fix buffer overrun in WindowsResourceCOFFWriter::writeSymbolTable()

Bob Haarman via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 18 14:10:14 PST 2017 

Author: inglorion
Date: Mon Dec 18 14:10:14 2017
New Revision: 321030

URL: http://llvm.org/viewvc/llvm-project?rev=321030&view=rev
Log:
Fix buffer overrun in WindowsResourceCOFFWriter::writeSymbolTable()

Summary:
We were using sprintf(..., "$R06X", <some uint32_t>) to create strings
that are expected to be exactly length 8, but this results in longer
strings if the uint32_t is greater than 0xffffff. This change modifies
the behavior as follows:

 - Uses the loop counter instead of the data offset. This gives us
   sequential symbol names, avoiding collisions as much as possible.

 - Masks the value to 0xffffff to avoid generating names longer than 8
   bytes.

 - Uses formatv instead of sprintf.

Fixes PR35581.

Reviewers: ruiu, zturner

Reviewed By: ruiu

Subscribers: hiraditya, llvm-commits

Differential Revision: https://reviews.llvm.org/D41270

Modified:
    llvm/trunk/lib/Object/WindowsResource.cpp
    llvm/trunk/test/tools/llvm-cvtres/machine.test
    llvm/trunk/test/tools/llvm-cvtres/symbols.test

Modified: llvm/trunk/lib/Object/WindowsResource.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/WindowsResource.cpp?rev=321030&r1=321029&r2=321030&view=diff
==============================================================================
--- llvm/trunk/lib/Object/WindowsResource.cpp (original)
+++ llvm/trunk/lib/Object/WindowsResource.cpp Mon Dec 18 14:10:14 2017
@@ -14,6 +14,7 @@
 #include "llvm/Object/WindowsResource.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/FileOutputBuffer.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include <ctime>
 #include <queue>
@@ -560,10 +561,9 @@ void WindowsResourceCOFFWriter::writeSym
 
   // Now write a symbol for each relocation.
   for (unsigned i = 0; i < Data.size(); i++) {
-    char RelocationName[9];
-    sprintf(RelocationName, "$R%06X", DataOffsets[i]);
+    auto RelocationName = formatv("$R{0:X-6}", i & 0xffffff).sstr<COFF::NameSize>();
     Symbol = reinterpret_cast<coff_symbol16 *>(BufferStart + CurrentOffset);
-    strncpy(Symbol->Name.ShortName, RelocationName, (size_t)COFF::NameSize);
+    memcpy(Symbol->Name.ShortName, RelocationName.data(), (size_t) COFF::NameSize);
     Symbol->Value = DataOffsets[i];
     Symbol->SectionNumber = 2;
     Symbol->Type = COFF::IMAGE_SYM_DTYPE_NULL;

Modified: llvm/trunk/test/tools/llvm-cvtres/machine.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/machine.test?rev=321030&r1=321029&r2=321030&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-cvtres/machine.test (original)
+++ llvm/trunk/test/tools/llvm-cvtres/machine.test Mon Dec 18 14:10:14 2017
@@ -29,46 +29,46 @@ X86:         Machine: IMAGE_FILE_MACHINE
 X86-DAG:   Relocations [
 X86-DAG:                 .rsrc$01 {
 X86-NEXT:      0x1E8 IMAGE_REL_I386_DIR32NB $R000000
-X86-NEXT:      0x198 IMAGE_REL_I386_DIR32NB $R000018
-X86-NEXT:      0x1A8 IMAGE_REL_I386_DIR32NB $R000340
-X86-NEXT:      0x1C8 IMAGE_REL_I386_DIR32NB $R000668
-X86-NEXT:      0x1D8 IMAGE_REL_I386_DIR32NB $R000698
-X86-NEXT:      0x1F8 IMAGE_REL_I386_DIR32NB $R000708
-X86-NEXT:      0x1B8 IMAGE_REL_I386_DIR32NB $R000720
-X86-NEXT:      0x188 IMAGE_REL_I386_DIR32NB $R000750
+X86-NEXT:      0x198 IMAGE_REL_I386_DIR32NB $R000001
+X86-NEXT:      0x1A8 IMAGE_REL_I386_DIR32NB $R000002
+X86-NEXT:      0x1C8 IMAGE_REL_I386_DIR32NB $R000003
+X86-NEXT:      0x1D8 IMAGE_REL_I386_DIR32NB $R000004
+X86-NEXT:      0x1F8 IMAGE_REL_I386_DIR32NB $R000005
+X86-NEXT:      0x1B8 IMAGE_REL_I386_DIR32NB $R000006
+X86-NEXT:      0x188 IMAGE_REL_I386_DIR32NB $R000007
 
 X64:         Machine: IMAGE_FILE_MACHINE_AMD64 (0x8664)
 X64-DAG:   Relocations [
 X64-DAG:                 .rsrc$01 {
 X64-NEXT:      0x1E8 IMAGE_REL_AMD64_ADDR32NB $R000000
-X64-NEXT:      0x198 IMAGE_REL_AMD64_ADDR32NB $R000018
-X64-NEXT:      0x1A8 IMAGE_REL_AMD64_ADDR32NB $R000340
-X64-NEXT:      0x1C8 IMAGE_REL_AMD64_ADDR32NB $R000668
-X64-NEXT:      0x1D8 IMAGE_REL_AMD64_ADDR32NB $R000698
-X64-NEXT:      0x1F8 IMAGE_REL_AMD64_ADDR32NB $R000708
-X64-NEXT:      0x1B8 IMAGE_REL_AMD64_ADDR32NB $R000720
-X64-NEXT:      0x188 IMAGE_REL_AMD64_ADDR32NB $R000750
+X64-NEXT:      0x198 IMAGE_REL_AMD64_ADDR32NB $R000001
+X64-NEXT:      0x1A8 IMAGE_REL_AMD64_ADDR32NB $R000002
+X64-NEXT:      0x1C8 IMAGE_REL_AMD64_ADDR32NB $R000003
+X64-NEXT:      0x1D8 IMAGE_REL_AMD64_ADDR32NB $R000004
+X64-NEXT:      0x1F8 IMAGE_REL_AMD64_ADDR32NB $R000005
+X64-NEXT:      0x1B8 IMAGE_REL_AMD64_ADDR32NB $R000006
+X64-NEXT:      0x188 IMAGE_REL_AMD64_ADDR32NB $R000007
 
 ARM:         Machine: IMAGE_FILE_MACHINE_ARMNT (0x1C4)
 ARM-DAG:   Relocations [
 ARM-DAG:                 .rsrc$01 {
 ARM-NEXT:      0x1E8 IMAGE_REL_ARM_ADDR32NB $R000000
-ARM-NEXT:      0x198 IMAGE_REL_ARM_ADDR32NB $R000018
-ARM-NEXT:      0x1A8 IMAGE_REL_ARM_ADDR32NB $R000340
-ARM-NEXT:      0x1C8 IMAGE_REL_ARM_ADDR32NB $R000668
-ARM-NEXT:      0x1D8 IMAGE_REL_ARM_ADDR32NB $R000698
-ARM-NEXT:      0x1F8 IMAGE_REL_ARM_ADDR32NB $R000708
-ARM-NEXT:      0x1B8 IMAGE_REL_ARM_ADDR32NB $R000720
-ARM-NEXT:      0x188 IMAGE_REL_ARM_ADDR32NB $R000750
+ARM-NEXT:      0x198 IMAGE_REL_ARM_ADDR32NB $R000001
+ARM-NEXT:      0x1A8 IMAGE_REL_ARM_ADDR32NB $R000002
+ARM-NEXT:      0x1C8 IMAGE_REL_ARM_ADDR32NB $R000003
+ARM-NEXT:      0x1D8 IMAGE_REL_ARM_ADDR32NB $R000004
+ARM-NEXT:      0x1F8 IMAGE_REL_ARM_ADDR32NB $R000005
+ARM-NEXT:      0x1B8 IMAGE_REL_ARM_ADDR32NB $R000006
+ARM-NEXT:      0x188 IMAGE_REL_ARM_ADDR32NB $R000007
 
 ARM64:       Machine: IMAGE_FILE_MACHINE_ARM64 (0xAA64)
 ARM64-DAG: Relocations [
 ARM64-DAG:               .rsrc$01 {
 ARM64-NEXT:    0x1E8 IMAGE_REL_ARM64_ADDR32NB $R000000
-ARM64-NEXT:    0x198 IMAGE_REL_ARM64_ADDR32NB $R000018
-ARM64-NEXT:    0x1A8 IMAGE_REL_ARM64_ADDR32NB $R000340
-ARM64-NEXT:    0x1C8 IMAGE_REL_ARM64_ADDR32NB $R000668
-ARM64-NEXT:    0x1D8 IMAGE_REL_ARM64_ADDR32NB $R000698
-ARM64-NEXT:    0x1F8 IMAGE_REL_ARM64_ADDR32NB $R000708
-ARM64-NEXT:    0x1B8 IMAGE_REL_ARM64_ADDR32NB $R000720
-ARM64-NEXT:    0x188 IMAGE_REL_ARM64_ADDR32NB $R000750
+ARM64-NEXT:    0x198 IMAGE_REL_ARM64_ADDR32NB $R000001
+ARM64-NEXT:    0x1A8 IMAGE_REL_ARM64_ADDR32NB $R000002
+ARM64-NEXT:    0x1C8 IMAGE_REL_ARM64_ADDR32NB $R000003
+ARM64-NEXT:    0x1D8 IMAGE_REL_ARM64_ADDR32NB $R000004
+ARM64-NEXT:    0x1F8 IMAGE_REL_ARM64_ADDR32NB $R000005
+ARM64-NEXT:    0x1B8 IMAGE_REL_ARM64_ADDR32NB $R000006
+ARM64-NEXT:    0x188 IMAGE_REL_ARM64_ADDR32NB $R000007

Modified: llvm/trunk/test/tools/llvm-cvtres/symbols.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/symbols.test?rev=321030&r1=321029&r2=321030&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-cvtres/symbols.test (original)
+++ llvm/trunk/test/tools/llvm-cvtres/symbols.test Mon Dec 18 14:10:14 2017
@@ -13,21 +13,21 @@ RUN: llvm-readobj -symbols %t | FileChec
 CHECK:        Name: $R000000
 CHECK-NEXT:    Value: 0
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000018
+CHECK:        Name: $R000001
 CHECK-NEXT:    Value: 24
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000340
+CHECK:        Name: $R000002
 CHECK-NEXT:    Value: 832
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000668
+CHECK:        Name: $R000003
 CHECK-NEXT:    Value: 1640
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000698
+CHECK:        Name: $R000004
 CHECK-NEXT:    Value: 1688
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000720
+CHECK:        Name: $R000006
 CHECK-NEXT:    Value: 1824
 CHECK-NEXT:    Section: .rsrc$02
-CHECK:        Name: $R000750
+CHECK:        Name: $R000007
 CHECK-NEXT:    Value: 1872
 CHECK-NEXT:    Section: .rsrc$02

More information about the llvm-commits mailing list