[Plastic-time]

Summary

The PE certificate decoder checks that a certificate's extensions are a SEQUENCE, but never looks inside it. Any entry is accepted, and so is the same extension listed twice.

No linked issue. Found while reading the decoder next to the other certificate-field fixes.

Problem and expected behavior

RFC 5280 §4.1 defines each entry:

Extension ::= SEQUENCE {
  extnID     OBJECT IDENTIFIER,
  critical   BOOLEAN DEFAULT FALSE,
  extnValue  OCTET STRING }

RFC 5280 §4.2 adds: "A certificate MUST NOT include more than one instance of a particular extension."

On main, a certificate breaking either rule is decoded and reported as usual. pyca/cryptography refuses the same certificates. It reports DuplicateExtension for a repeat and a parse error for a malformed entry.

Change and scope

• New requireExtensions runs on the extensions SEQUENCE in finishTbsCertificate.
• Each entry must be an OID, an optional one-octet BOOLEAN, and an OCTET STRING, with nothing else.
• A repeated extnID fails with, for example, "TBSCertificate repeats extension 2.5.29.14."
• Only BER-level rules are added, like the INTEGER checks in the same file. DER-only rules stay out: an explicit critical FALSE and a TRUE other than 0xFF are still accepted.
• Extension values are still not decoded or reported.

Follow the contribution scope. Ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

• Semantic owner and earliest changed stage: finishTbsCertificate in src/native/pe/PeCertificateAsn1.ts.
• Contribution scope fit and linked maintainer decision: ordinary parser fix; not applicable.
• CLI and MCP/tool-catalog contract: unchanged schemas. Such a certificate now fails as invalid_request, like other malformed certificate fields.
• Provider, bridge, target-format, or platform compatibility: none.
• Evidence, artifact, provenance, or reconstruction contract: well-formed certificates are reported exactly as before.
• Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
• Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

• Read existing files or captured data.
• Launch or attach to a process.
• Inject instrumentation or run scripts/code.
• Change process memory, registers, files, or configuration.
• Contact network services.
• Recover protected code or work around protections.
• Documentation or usability only; no target interaction.
• Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

• Tests added or updated: src/native/pe/PeCertificateParser.test.ts › "rejects malformed and repeated certificate extensions". It refuses eight malformed entries and a repeat that is not adjacent. It accepts two distinct extensions, one marked critical.
• Base reproduction: the 6 test files listed below give 2 failed / 84 passed with only src/native/pe/PeCertificateAsn1.ts reset to main (d896ab7). On this branch they give 1 failed / 85 passed. The remaining failure is tests/boundary/filesystem/peCertificates.test.ts › "reports non-file and malformed PE selections as caller input errors". It is EPERM creating a symlink on this Windows host, and it fails the same way on main.
• User-visible CLI/MCP output: take the repository's pkcs7SignedDataFixture() and list its subjectKeyIdentifier extension twice. On main, inspect-pe-certificates --json exits 0 and reports the certificate with serial 11223344. On this branch it exits 1 with invalid_request and PKCS#7 signedData is malformed: TBSCertificate repeats extension 2.5.29.14.
• Independent oracle: 16 certificates that differ only in their extensions, loaded with pyca/cryptography 50.0.2 (load_der_x509_certificate, then .extensions).
  • main accepts all 16. cryptography refuses 12.
  • This branch agrees with cryptography on 15.
  • The one difference is an explicit critical FALSE. cryptography refuses it as an encoded DER default. This branch keeps accepting it on purpose (see scope).
• Real binaries: on 60 signed binaries from Program Files (one per distinct signer certificate), the results match main exactly. 49 decode, with 117 certificates, and every one passes the new check. The same 11 fail as on main: 8 hit the ASN.1 nesting limit that fix(pe): admit Authenticode nesting depth of signed Windows binaries #7160 addresses, and 3 carry vendor bytes after the PKCS#7. 2 are catalog-signed, so Windows reports a different signer.
• Remaining proof gaps: Windows only. An empty extensions SEQUENCE is still accepted. RFC 5280 asks for one or more, but cryptography accepts it too, so this PR leaves it alone.

For evidence-bearing changes:

• Observed, derived, and inferred claims remain distinguishable.
• Artifact identity, source provenance, and failed attempts remain preserved.
• Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Windows 11 x64 (10.0.26200), Node 24.22.1, base main d896ab7.

• npm run test:focused -- over src/native/pe/PeCertificateParser.test.ts, src/application/binaryDiagnostics/PeCertificatesService.test.ts, src/domain/native/peCertificates.test.ts, tests/acceptance/peCertificatesCli.test.ts, tests/boundary/filesystem/peCertificates.test.ts and tests/boundary/mcp/peCertificatesMcp.test.ts — 85 passed, 1 failed (EPERM symlink, same on main). With the source reset to main: 2 failed / 84 passed.
• npm run check:fast (typecheck and lint) — passed. oxfmt --check on both files — passed.
• Compiled CLI on the fixture above, on main and on this branch — results as shown.
• git merge-tree with fix(pe): reject certificate versions and fields RFC 5280 does not allow #8921, fix(pe): reject a SignerInfo version that does not match its sid #8924, fix(pe): preserve certificate serial number sign #8916 and fix(pe): admit Authenticode nesting depth of signed Windows binaries #7160, which edit the same files: all merge cleanly.

Compatibility, safety, and release

• Breaking changes or migration steps: none for well-formed certificates. A certificate with a malformed or repeated extension is now rejected instead of reported.
• Real Hopper/Ghidra, browser, or OS coverage: not applicable; Windows only.
• Package or release metadata impact: none.
• Security, privacy, process, or containment review: parser-only change; no new I/O, process or network behavior.

Prepared with AI assistance.

Author readiness

Human owner: @Plastic-time

• 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.
• I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
• I declare this PR ready for initial maintainer review.

Before merge

• This PR addresses a concrete problem or an agreed enhancement.
• Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
• The PR has one focused outcome and the title follows type(scope): outcome.
• Related issue is linked, or the reason for not linking one is stated above.
• Tests cover changed observable behavior and meaningful failure paths.
• Owning docs, contracts, and generated metadata are updated where needed.
• User-visible CLI/MCP changes include representative output.
• I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-11-04T18:04:56.313392Z We couldn’t complete this review. Comment @codex review to try again.	9ec77cd	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds extension checks at the shared X.509 decoder boundary.

• PE certificate parsing rejects malformed or repeated extensions.

Plastic-time explicitly leaves empty extension lists accepted, allows explicit critical FALSE and non-0xFF TRUE values, and defers decoding extension values. These are intentional scope choices.

Supersession could not be checked on GitHub because authentication was unavailable. Local history showed no replacement change. Tests were not run during this review.

_{Reviews (1) · Last reviewed commit: "fix(pe): reject malformed and repeated c..." · Reviewed by Greptile}