[PR #6711] [CLOSED] fix(pe): accept WIN_CERTIFICATE padding counted in dwLength #7397

New issue

Closed

opened 2026-11-01 21:47:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6711
Author: @Deepak-Kambala
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/pe-certificate-dwlength-padding

📝 Commits (1)

fb2db30 fix(pe): accept WIN_CERTIFICATE padding counted in dwLength

📊 Changes

3 files changed (+72 additions, -4 deletions)

View changed files

📝 src/native/pe/PeCertificateParser.test.ts (+52 -2)
📝 src/native/pe/PeCertificateParser.ts (+18 -1)
📝 src/native/pe/PeCertificates.fixture.ts (+2 -1)

📄 Description

inspect-pe-certificates rejected valid PE32 and PE32+ certificate entries when WIN_CERTIFICATE.dwLength included the required zero alignment padding. Those bytes were passed to the CMS parser as encoded payload, which rejected them as trailing ASN.1 data.

Use the ASN.1 root end offset to bound the encoded CMS payload. Accept trailing bytes only when they are zero-filled and exactly the padding needed to align the full certificate entry. Preserve the original artifact bytes and digest, dwLength, and entry range; report the CMS payload range and hash without alignment padding.

Regression coverage checks counted-padding PE32 and PE32+ fixtures, existing controls, invalid trailing bytes, malformed CMS, truncated entries, and length boundaries. It also checks artifact identity and entry/payload ranges and hashes.

Fixes #6694

Verification

Parser tests: 30 passed.
Related PE tests: 76 passed.
Focused CLI/filesystem/MCP/contract tests: 17 passed, 4 skipped.
npm run check:fast: passed.
git diff --check: passed.

Provider and artifact coverage

Tests use synthetic PE fixtures; no external provider or real-world PE artifact was verified.

Author readiness

Human owner:

[yes] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
[yes] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
[yes] I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6711 **Author:** [@Deepak-Kambala](https://github.com/Deepak-Kambala) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/pe-certificate-dwlength-padding` --- ### 📝 Commits (1) - [`fb2db30`](https://github.com/morluto/rea/commit/fb2db3021f5aedbd66b147744e3379ef3cdc1f98) fix(pe): accept WIN_CERTIFICATE padding counted in dwLength ### 📊 Changes **3 files changed** (+72 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/native/pe/PeCertificateParser.test.ts` (+52 -2) 📝 `src/native/pe/PeCertificateParser.ts` (+18 -1) 📝 `src/native/pe/PeCertificates.fixture.ts` (+2 -1) </details> ### 📄 Description `inspect-pe-certificates` rejected valid PE32 and PE32+ certificate entries when `WIN_CERTIFICATE.dwLength` included the required zero alignment padding. Those bytes were passed to the CMS parser as encoded payload, which rejected them as trailing ASN.1 data. Use the ASN.1 root end offset to bound the encoded CMS payload. Accept trailing bytes only when they are zero-filled and exactly the padding needed to align the full certificate entry. Preserve the original artifact bytes and digest, `dwLength`, and entry range; report the CMS payload range and hash without alignment padding. Regression coverage checks counted-padding PE32 and PE32+ fixtures, existing controls, invalid trailing bytes, malformed CMS, truncated entries, and length boundaries. It also checks artifact identity and entry/payload ranges and hashes. Fixes #6694 ## Verification - Parser tests: 30 passed. - Related PE tests: 76 passed. - Focused CLI/filesystem/MCP/contract tests: 17 passed, 4 skipped. - `npm run check:fast`: passed. - `git diff --check`: passed. ## Provider and artifact coverage Tests use synthetic PE fixtures; no external provider or real-world PE artifact was verified. ## Author readiness Human owner:  - [yes] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [yes] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [yes] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:33 +00:00