diff --git a/CHANGELOG.md b/CHANGELOG.md index 419ea0b..23bd5a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,16 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Added -- Unit test for bound imports directory. +- Unit test for bound imports directory [#18](https://github.com/saferwall/pe/pull/18) +- Unit test for security directory [#19](https://github.com/saferwall/pe/pull/19). ## Changed -- Make GetData() and GetRVAFromOffset() helper routines public. +- Make `GetData()` and `GetRVAFromOffset()` helper routines public. ## Fixed - Imphash calculation [#17](https://github.com/saferwall/pe/pull/17) thanks to [@secDre4mer](https://github.com/secDre4mer). - +- Null certificate header in security directory [#19](https://github.com/saferwall/pe/pull/19) ## [1.1.0] - 2021-12-20 diff --git a/boundimports.go b/boundimports.go index 29f6321..edbc99b 100644 --- a/boundimports.go +++ b/boundimports.go @@ -5,7 +5,6 @@ package pe import ( - "bytes" "encoding/binary" "log" ) @@ -109,8 +108,7 @@ func (pe *File) parseBoundImportDirectory(rva, size uint32) (err error) { var forwarderRefs []BoundForwardedRefData for i := uint32(0); i < count; i++ { - buf := bytes.NewReader(pe.data[rva : rva+bndFrwdRefSize]) - err := binary.Read(buf, binary.LittleEndian, &bndFrwdRef) + err = pe.structUnpack(&bndFrwdRef, rva, bndFrwdRefSize) if err != nil { return err } diff --git a/security.go b/security.go index 9b41652..34bc087 100644 --- a/security.go +++ b/security.go @@ -49,8 +49,10 @@ const ( ) var ( - errSecurityDataDirOutOfBands = errors.New( - `boundary checks failed in security data directory`) + // ErrSecurityDataDirInvalidCertHeader is reported when the certificate + // header in the security directory is invalid. + ErrSecurityDataDirInvalid = errors.New( + `invalid certificate header in security directory`) ) // Certificate directory. @@ -216,11 +218,15 @@ func (pe *File) parseSecurityDirectory(rva, size uint32) error { for { err := pe.structUnpack(&certHeader, fileOffset, certSize) if err != nil { - return errSecurityDataDirOutOfBands + return ErrOutsideBoundary } if fileOffset+certHeader.Length > pe.size { - return errSecurityDataDirOutOfBands + return ErrOutsideBoundary + } + + if certHeader.Length == 0 { + return ErrSecurityDataDirInvalid } certContent := pe.data[fileOffset+certSize : fileOffset+certHeader.Length] diff --git a/security_test.go b/security_test.go index f4c25e9..5dcab06 100644 --- a/security_test.go +++ b/security_test.go @@ -6,9 +6,94 @@ package pe import ( "fmt" + "reflect" "testing" + "time" ) +type TestSecurityEntry struct { + Header WinCertificate + Info CertInfo + Verified bool + err error +} + +func TestParseSecurityDirectory(t *testing.T) { + + tests := []struct { + in string + out TestSecurityEntry + }{ + { + getAbsoluteFilePath("test/putty.exe"), + TestSecurityEntry{ + Header: WinCertificate{ + Length: 0x3D90, + Revision: 0x200, + CertificateType: 0x2, + }, + Info: CertInfo{ + Issuer: "GB, Greater Manchester, Salford, COMODO RSA Code Signing CA", + Subject: "GB, Cambridgeshire, Cambridge, Simon Tatham, Simon Tatham", + NotBefore: time.Date(2018, time.November, 13, 00, 00, 0, 0, time.UTC), + NotAfter: time.Date(2021, time.November, 8, 23, 59, 59, 0, time.UTC), + }, + err: nil, + }, + }, + { + getAbsoluteFilePath("test/00121dae38f26a33da2990987db58738c5a5966930126a42f606a3b40e014624"), + TestSecurityEntry{ + err: ErrSecurityDataDirInvalid, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + ops := Options{Fast: true} + file, err := New(tt.in, &ops) + if err != nil { + t.Fatalf("New(%s) failed, reason: %v", tt.in, err) + } + + err = file.Parse() + if err != nil { + t.Fatalf("Parse(%s) failed, reason: %v", tt.in, err) + } + + var va, size uint32 + if file.Is64 { + oh64 := file.NtHeader.OptionalHeader.(ImageOptionalHeader64) + dirEntry := oh64.DataDirectory[ImageDirectoryEntryCertificate] + va = dirEntry.VirtualAddress + size = dirEntry.Size + } else { + oh32 := file.NtHeader.OptionalHeader.(ImageOptionalHeader32) + dirEntry := oh32.DataDirectory[ImageDirectoryEntryCertificate] + va = dirEntry.VirtualAddress + size = dirEntry.Size + } + + err = file.parseSecurityDirectory(va, size) + if err != tt.out.err { + t.Fatalf("parseSecurityDirectory(%s) failed, reason: %v", tt.in, err) + } + + got := file.Certificates + if tt.out.err == nil { + if !reflect.DeepEqual(got.Header, tt.out.Header) { + t.Fatalf("certificate header assertion failed, got %v, want %v", got.Header, tt.out.Header) + } + if !reflect.DeepEqual(got.Info, tt.out.Info) { + t.Fatalf("certificate info assertion failed, got %v, want %v", got.Info, tt.out.Info) + } + } + + }) + } +} + func TestAuthentihash(t *testing.T) { tests := []struct { diff --git a/test/00121dae38f26a33da2990987db58738c5a5966930126a42f606a3b40e014624.7z b/test/00121dae38f26a33da2990987db58738c5a5966930126a42f606a3b40e014624.7z new file mode 100644 index 0000000..d74b725 Binary files /dev/null and b/test/00121dae38f26a33da2990987db58738c5a5966930126a42f606a3b40e014624.7z differ