Skip to content

Commit

Permalink
Fix buffer overrun in WindowsResourceCOFFWriter::writeSymbolTable()
Browse files Browse the repository at this point in the history 
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

llvm-svn: 321030
  • Loading branch information
@inglorion
inglorion committed Dec 18, 2017
1 parent 8f3c351 commit ea5ff9f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 37 deletions.
6 changes: 3 additions & 3 deletions llvm/lib/Object/WindowsResource.cpp
View file
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down
56 changes: 28 additions & 28 deletions llvm/test/tools/llvm-cvtres/machine.test
View file
Expand Up @@ -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
12 changes: 6 additions & 6 deletions llvm/test/tools/llvm-cvtres/symbols.test
View file
Expand Up @@ -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

0 comments on commit ea5ff9f

Please sign in to comment.