Merge pull request #19 from saferwall/noteworthy

feat: unit test security directory

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
 

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
 			}

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]

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 {