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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@321030 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Object/WindowsResource.cpp b/lib/Object/WindowsResource.cpp
index 9ca584a4a1ae15a26b090ba9a804f949a8774aa4..271224ec6312009ee9e43b2b35f8404c8a4cc6b7 100644 (file)
--- a/lib/Object/WindowsResource.cpp
+++ b/lib/Object/WindowsResource.cpp
@@ -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::writeSymbolTable() {
 
   // 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;

diff --git a/test/tools/llvm-cvtres/machine.test b/test/tools/llvm-cvtres/machine.test
index cac36062f301acd7ff66bc51c6296d0927dcea71..5ce6cbaf697d5e60d3a69e93914a1268c17a6237 100644 (file)
--- a/test/tools/llvm-cvtres/machine.test
+++ b/test/tools/llvm-cvtres/machine.test
@@ -29,46 +29,46 @@ X86:         Machine: IMAGE_FILE_MACHINE_I386 (0x14C)
 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

diff --git a/test/tools/llvm-cvtres/symbols.test b/test/tools/llvm-cvtres/symbols.test
index 2ca3a193ac404f9fd33dbb0bd9821da9d4b064ad..14f5c360d454953b3fc44d26c4371bfd2f8894a7 100644 (file)
--- a/test/tools/llvm-cvtres/symbols.test
+++ b/test/tools/llvm-cvtres/symbols.test
@@ -13,21 +13,21 @@ RUN: llvm-readobj -symbols %t | FileCheck %s
 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