From c5aeee57fd1095c37588086397a1bee522b22094 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Tue, 31 Aug 2021 05:09:24 -0700 Subject: [PATCH] Fix read data out of boundary cause System.AccessViolationException (#470) * Fix read data out of boundary cause System.AccessViolationException * Add in history * fix load error for special gcc machine type * Update src/ReleaseHistory.md Co-authored-by: Eddy Nakamura --- .../ElfBinary/Dwarf/DwarfLineNumberProgram.cs | 6 + .../ElfBinary/Dwarf/DwarfMemoryReader.cs | 20 ++ src/ReleaseHistory.md | 1 + .../Elf/ElfBinaryTests.cs | 230 ++++++++++-------- .../gcc.tls.aarch64-linux-gnu-gcc-8.o | Bin 0 -> 2344 bytes .../gcc.tls.mips-linux-gnu-gcc-9.o | Bin 0 -> 2824 bytes 6 files changed, 153 insertions(+), 104 deletions(-) create mode 100644 src/Test.UnitTests.BinaryParsers/TestsData/Dwarf/MachineType/gcc.tls.aarch64-linux-gnu-gcc-8.o create mode 100644 src/Test.UnitTests.BinaryParsers/TestsData/Dwarf/MachineType/gcc.tls.mips-linux-gnu-gcc-9.o diff --git a/src/BinaryParsers/ElfBinary/Dwarf/DwarfLineNumberProgram.cs b/src/BinaryParsers/ElfBinary/Dwarf/DwarfLineNumberProgram.cs index e81500a85..f981e33fa 100644 --- a/src/BinaryParsers/ElfBinary/Dwarf/DwarfLineNumberProgram.cs +++ b/src/BinaryParsers/ElfBinary/Dwarf/DwarfLineNumberProgram.cs @@ -179,6 +179,12 @@ private static List ReadData(DwarfMemoryReader debugLine, // Read header ulong length = debugLine.ReadLength(out bool is64bit); int endPosition = debugLine.Position + (int)length; + + if (endPosition > debugLine.Data.Length - 1) + { + endPosition = debugLine.Data.Length - 1; + } + debugLine.ReadUshort(); // version debugLine.ReadOffset(is64bit); // headerLength byte minimumInstructionLength = debugLine.ReadByte(); diff --git a/src/BinaryParsers/ElfBinary/Dwarf/DwarfMemoryReader.cs b/src/BinaryParsers/ElfBinary/Dwarf/DwarfMemoryReader.cs index bbb4bc562..e44755c1d 100644 --- a/src/BinaryParsers/ElfBinary/Dwarf/DwarfMemoryReader.cs +++ b/src/BinaryParsers/ElfBinary/Dwarf/DwarfMemoryReader.cs @@ -249,6 +249,11 @@ public byte[] ReadBlock(uint size) /// The position. public byte[] ReadBlock(uint size, int position) { + if (position >= Data.Length) + { + return Array.Empty(); + } + int originalPosition = Position; Position = position; byte[] result = ReadBlock(size); @@ -262,6 +267,11 @@ public byte[] ReadBlock(uint size, int position) /// The position. public string ReadString(int position) { + if (position >= Data.Length) + { + return string.Empty; + } + int originalPosition = Position; Position = position; string result = ReadString(); @@ -275,6 +285,11 @@ public string ReadString(int position) /// The position. public uint ReadUint(int position) { + if (position >= Data.Length) + { + return 0; + } + int originalPosition = Position; Position = position; uint result = ReadUint(); @@ -289,6 +304,11 @@ public uint ReadUint(int position) /// The position. public T ReadStructure(int position) { + if (position >= Data.Length) + { + return default; + } + int originalPosition = Position; Position = position; T result = ReadStructure(); diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 3000b677d..6ca370503 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -2,6 +2,7 @@ ## Unreleased +* BUGFIX: Fix exception `System.AccessViolationException` caused by trying to read data out of boundary. [#470](https://github.com/microsoft/binskim/pull/470) * BUGFIX: Fix exception handling when PDB cannot be loaded by `IDiaDataSource`. [#461](https://github.com/microsoft/binskim/pull/461) * BREAKING: PDB exceptions will be reported once per target. [#465](https://github.com/microsoft/binskim/pull/465) diff --git a/src/Test.UnitTests.BinaryParsers/Elf/ElfBinaryTests.cs b/src/Test.UnitTests.BinaryParsers/Elf/ElfBinaryTests.cs index 934b6c762..47f013838 100644 --- a/src/Test.UnitTests.BinaryParsers/Elf/ElfBinaryTests.cs +++ b/src/Test.UnitTests.BinaryParsers/Elf/ElfBinaryTests.cs @@ -29,112 +29,25 @@ internal static string GetTestDirectory(string relativeDirectory) } [Fact] - public void ValidateDwarfV4_WithO2() - { - // Hello.c compiled using: gcc -Wall -O2 -g -gdwarf-4 hello.c -o hello4 - string fileName = Path.Combine(TestData, "Dwarf/hello-dwarf4-o2"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(4); - binary.CommandLineInfos.Should().OnlyContain(x => x.CommandLine.Contains("O2")); - binary.GetLanguage().Should().Be(DwarfLanguage.C99); - } - - [Fact] - public void ValidateDwarfV5_WithO2() - { - // Hello.c compiled using: gcc -Wall -O2 -g -gdwarf-5 hello.c -o hello5 - string fileName = Path.Combine(TestData, "Dwarf/hello-dwarf5-o2"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(5); - binary.CommandLineInfos.Should().OnlyContain(x => x.CommandLine.Contains("O2")); - binary.GetLanguage().Should().Be(DwarfLanguage.C11); - } - - [Fact] - public void ValidateDwarfV4_WithO2_Split_DebugFileExists() - { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o - string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV4/dwotest.gcc.4.o"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(4); - binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); - } - - [Fact] - public void ValidateDwarfV5_WithO2_Split_DebugFileExists() - { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-5 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.5.o - string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV5/dwotest.gcc.5.o"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(5); - binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus14); - } - - [Fact] - public void ValidateDwarfV4_WithO2_Split_DebugFileMissing() - { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o - string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV4DebugFileMissing/dwotest.gcc.4.o"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(4); - binary.GetLanguage().Should().Be(DwarfLanguage.Unknown); //missing dwo file should not cause exception - } - - [Fact] - public void ValidateDwarfV5_WithO2_Split_DebugFileMissing() - { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-5 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.5.o - string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV5DebugFileMissing/dwotest.gcc.5.o"); - using var binary = new ElfBinary(new Uri(fileName)); - binary.DwarfVersion.Should().Be(5); - binary.GetLanguage().Should().Be(DwarfLanguage.Unknown); //missing dwo file should not cause exception - } - - [Fact] - public void ValidateDwarfV4_WithO2_Split_DebugFileInAnotherDirectory() - { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o - string fileName = Path.Combine(TestData, - "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/BinaryDirectory/dwotest.gcc.4.o"); - - // test for: when not pass in directories - using var binaryWithoutPathList = new ElfBinary(new Uri(fileName)); - binaryWithoutPathList.GetLanguage().Should().Be(DwarfLanguage.Unknown); - - // test for: when able to find in any of the pass in directories - string localSymbolDirectory1 = Path.Combine(TestData, - "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/NotExists"); - string localSymbolDirectory2 = Path.Combine(TestData, - "WithoutDwoFiles"); - string localSymbolDirectory3 = Path.Combine(TestData, - "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/AnotherLocalSymbolDirectory"); - var pathListFound = new List() { localSymbolDirectory1, localSymbolDirectory2, localSymbolDirectory3 }; - using var binaryFound = new ElfBinary(new Uri(fileName), string.Join(';', pathListFound)); - binaryFound.DwarfVersion.Should().Be(4); - binaryFound.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); - - // test for: when not able to find in any of the pass in directories, also not able to find in same directory - var pathListNotFound = new List() { localSymbolDirectory1, localSymbolDirectory2 }; - using var binaryNotFound = new ElfBinary(new Uri(fileName), string.Join(';', pathListNotFound)); - binaryNotFound.GetLanguage().Should().Be(DwarfLanguage.Unknown); - } - - [Fact] - public void ValidateDwarfV4_WithO2_Split_DebugFileInSameDirectory() + public void Validate_MachineType() { - // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o - string fileName = Path.Combine(TestData, - "Dwarf/DwarfSplitV4/dwotest.gcc.4.o"); + // test for: build with aarch64, should load without error + // compiled using: + // aarch64-linux-gnu-gcc-8 tls.c -c -o gcc.tls.aarch64-linux-gnu-gcc-8.o -mlittle-endian + // -mabi=lp64 -g -O2 -std=gnu11 -fgnu89-inline -feliminate-unused-debug-types + // -fmerge-all-constants -fno-stack-protector -frounding-math -fno-math-errno -ftls-model=initial-exec + string fileName = Path.Combine(TestData, "Dwarf/MachineType/gcc.tls.aarch64-linux-gnu-gcc-8.o"); + using var binaryAarch64 = new ElfBinary(new Uri(fileName)); + binaryAarch64.ELF.Machine.Should().Be(ELFSharp.ELF.Machine.AArch64); - // test for: when not able to find in any of the pass in directories, should try load in same directory - string localSymbolDirectory1 = Path.Combine(TestData, - "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/NotExists"); - string localSymbolDirectory2 = Path.Combine(TestData, - "WithoutDwoFiles"); - var pathList = new List() { localSymbolDirectory1, localSymbolDirectory2 }; - using var binary = new ElfBinary(new Uri(fileName), string.Join(';', pathList)); - binary.DwarfVersion.Should().Be(4); - binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); + // test for: build with mips, should load without error + // compiled using: + // mips-linux-gnu-gcc-9 tls.c -c -o gcc.tls.mips-linux-gnu-gcc-9.o + // -mabi=64 -g -O2 -std=gnu11 -fgnu89-inline -feliminate-unused-debug-types + // -fmerge-all-constants -fno-stack-protector -frounding-math -fno-math-errno -ftls-model=initial-exec + fileName = Path.Combine(TestData, "Dwarf/MachineType/gcc.tls.mips-linux-gnu-gcc-9.o"); + using var binaryMips = new ElfBinary(new Uri(fileName)); + binaryMips.ELF.Machine.Should().Be(ELFSharp.ELF.Machine.MIPS); } [Fact] @@ -248,5 +161,114 @@ public void Validate_DebugFileType() binaryDebugOnlyFileDwo.DebugFileType.Should().Be(DebugFileType.DebugOnlyFileDwo); binaryDebugOnlyFileDwo.DebugFileLoaded.Should().Be(false); } + + [Fact] + public void ValidateDwarfV4_WithO2() + { + // Hello.c compiled using: gcc -Wall -O2 -g -gdwarf-4 hello.c -o hello4 + string fileName = Path.Combine(TestData, "Dwarf/hello-dwarf4-o2"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(4); + binary.CommandLineInfos.Should().OnlyContain(x => x.CommandLine.Contains("O2")); + binary.GetLanguage().Should().Be(DwarfLanguage.C99); + } + + [Fact] + public void ValidateDwarfV5_WithO2() + { + // Hello.c compiled using: gcc -Wall -O2 -g -gdwarf-5 hello.c -o hello5 + string fileName = Path.Combine(TestData, "Dwarf/hello-dwarf5-o2"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(5); + binary.CommandLineInfos.Should().OnlyContain(x => x.CommandLine.Contains("O2")); + binary.GetLanguage().Should().Be(DwarfLanguage.C11); + } + + [Fact] + public void ValidateDwarfV4_WithO2_Split_DebugFileExists() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o + string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV4/dwotest.gcc.4.o"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(4); + binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); + } + + [Fact] + public void ValidateDwarfV5_WithO2_Split_DebugFileExists() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-5 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.5.o + string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV5/dwotest.gcc.5.o"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(5); + binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus14); + } + + [Fact] + public void ValidateDwarfV4_WithO2_Split_DebugFileMissing() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o + string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV4DebugFileMissing/dwotest.gcc.4.o"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(4); + binary.GetLanguage().Should().Be(DwarfLanguage.Unknown); //missing dwo file should not cause exception + } + + [Fact] + public void ValidateDwarfV5_WithO2_Split_DebugFileMissing() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-5 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.5.o + string fileName = Path.Combine(TestData, "Dwarf/DwarfSplitV5DebugFileMissing/dwotest.gcc.5.o"); + using var binary = new ElfBinary(new Uri(fileName)); + binary.DwarfVersion.Should().Be(5); + binary.GetLanguage().Should().Be(DwarfLanguage.Unknown); //missing dwo file should not cause exception + } + + [Fact] + public void ValidateDwarfV4_WithO2_Split_DebugFileInSameDirectory() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o + string fileName = Path.Combine(TestData, + "Dwarf/DwarfSplitV4/dwotest.gcc.4.o"); + + // test for: when not able to find in any of the pass in directories, should try load in same directory + string localSymbolDirectory1 = Path.Combine(TestData, + "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/NotExists"); + string localSymbolDirectory2 = Path.Combine(TestData, + "WithoutDwoFiles"); + var pathList = new List() { localSymbolDirectory1, localSymbolDirectory2 }; + using var binary = new ElfBinary(new Uri(fileName), string.Join(';', pathList)); + binary.DwarfVersion.Should().Be(4); + binary.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); + } + + [Fact] + public void ValidateDwarfV4_WithO2_Split_DebugFileInAnotherDirectory() + { + // dwotest.cpp compiled using: gcc -Wall -O2 -g -gdwarf-4 dwotest.cpp -gsplit-dwarf -o dwotest.gcc.4.o + string fileName = Path.Combine(TestData, + "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/BinaryDirectory/dwotest.gcc.4.o"); + + // test for: when not pass in directories + using var binaryWithoutPathList = new ElfBinary(new Uri(fileName)); + binaryWithoutPathList.GetLanguage().Should().Be(DwarfLanguage.Unknown); + + // test for: when able to find in any of the pass in directories + string localSymbolDirectory1 = Path.Combine(TestData, + "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/NotExists"); + string localSymbolDirectory2 = Path.Combine(TestData, + "WithoutDwoFiles"); + string localSymbolDirectory3 = Path.Combine(TestData, + "Dwarf/DwarfSplitV4DebugFileInAnotherDirectory/AnotherLocalSymbolDirectory"); + var pathListFound = new List() { localSymbolDirectory1, localSymbolDirectory2, localSymbolDirectory3 }; + using var binaryFound = new ElfBinary(new Uri(fileName), string.Join(';', pathListFound)); + binaryFound.DwarfVersion.Should().Be(4); + binaryFound.GetLanguage().Should().Be(DwarfLanguage.CPlusPlus); + + // test for: when not able to find in any of the pass in directories, also not able to find in same directory + var pathListNotFound = new List() { localSymbolDirectory1, localSymbolDirectory2 }; + using var binaryNotFound = new ElfBinary(new Uri(fileName), string.Join(';', pathListNotFound)); + binaryNotFound.GetLanguage().Should().Be(DwarfLanguage.Unknown); + } } } diff --git a/src/Test.UnitTests.BinaryParsers/TestsData/Dwarf/MachineType/gcc.tls.aarch64-linux-gnu-gcc-8.o b/src/Test.UnitTests.BinaryParsers/TestsData/Dwarf/MachineType/gcc.tls.aarch64-linux-gnu-gcc-8.o new file mode 100644 index 0000000000000000000000000000000000000000..4c0c3f0f9d8d24309464bfcca0b397cac114d1e7 GIT binary patch literal 2344 zcmbtV&2AGh5T4DZr4SlgRZvhl1XO-1>P=e;N{b2+1qxCH2&v*&Z4x(G*m$E|FSG~5 zjYr_hD{$ioFN4IDGY6Pi+v&zxE-+PgX1@8x9@}I0{lncyCC33H4t#-Q&!7N5%Exvy zGn+6AGjIy+8~9V)oGk6TML%qfD99_ou$-!QJ>et zWKbW((JS7TNj*(?T<>zhV_)(<{>5<=!_L#a+V<*dt>LYED>c>+11Uq!xab7FK;Lf# zTjAjLdX05!?Acn4C9<>C6)C2$1JsQT7Kktq99?J{{Z;0cCt z#x7@m7_xRG66p&`sEBY#zx|qFp%M#?Vg%zT#R5bZ=a)T2B8y?oB0?V*%KA}PCExKP}DZr1%p{K`0phzrwy!AEXP#-V$4E?58X`l+s7>_Cw5CiRFV+~|Cj zW<M)LEH?3$Mx^|8mmOJuBc&sdTlO7J?q z^ucQ-33yW3F%Nw&yB+(1IEau!H)Y>%wPOATyj<9iebMEZsTg$AjE$3k*N*yqj&F=7 z@b-9ko7HVHMgNChz+BoYvu-m~(-@Z4+d4!qp+%V8!ObCK)UfmE0j2Z(*ZgJ7&E_9A zlA5onRIbw$V9}s~uw4@|A1FH1wKmG;-oyZn*)#;Lzv%@XYQKt_Zl~$x^BSjl>@g{n z=s2w}FE;lj25pS^pL9}9Yol!L3k($1-yjgh)JgSI?RNbKh}mNz1YhZca#g9}*XG6; zA#7#HP{w!duUQGkHToU$it^|6Q%nED^J4Oh{(wPa=Xv*wpXxC#c~l<=!+g%(8H6qwm=h$0dcSQJ4U2?|nGsRvMHdndaF|5({$ zl%9wX5?4-$Gj}9T962KX42~SQaNxute9xZQ9Xr_w5R-V`d*8h8y_x4X-p}88?_Jk* z*laj#1-b>yR-i2Xa>Upkj= z7s$p1n#=KPtGyH^3fsKwT<@-RTHUMN)n^%VH(6_Cr}IW? zu}oynqynO3T4siUR8~~S3qNVexjRXJl3;a6^@LG!-AH_EOe$KXy4t&+DS4=v z7a^X>!9Ha+&kdEtQVU6 ziN^7ZQX7U(_&y#}znT+_qb8tj`~Kbo_HX&N|3`CvyIH@ZQ}PjH=bzyG&uhP?_)-Hq zjRiY@fHf0uSodo`)b`hqPvSYRZM%NzjQl@of1#xjcK)x<;Qy`(Jww#e=6?Zf(f)ta z{`8({|K|H`@Bf>{`5ohbL&q_~&i|dozik{U>1=5EN*$l|3&r^!kBQgdyB}-b^)vWC zrpL#BHrBQ2Qf+9gR$?}rd<2l*yf@m>{kBq^A0XH8nBNt} literal 0 HcmV?d00001