fix(certificates)!: disclose undecoded unsigned attributes (#7920)

* fix(certificates): disclose undecoded unsigned attributes

* test(certificates): use parser-owned limitation text
This commit is contained in:
morluto authored and GitHub committed 2026-10-29 08:56:07 +08:00
1 parent 0386efd1e1
commit 33d6975749
11 files changed
+183 -9

No files matched your search

+18 -1
View File
@@ -445,6 +445,7 @@ export interface ParsedSigner {
readonly serial_hex: string | null;
readonly subject_key_identifier_hex: string | null;
readonly authenticated_attributes: boolean;
readonly unsigned_attribute_oids: readonly string[] | null;
}
const signerIdentifier = (
@@ -517,7 +518,22 @@ const parseSigner = (
if (signature.content.length === 0)
return fail("SignerInfo signature OCTET STRING is empty.");
cursor += 2;
if (children[cursor]?.tag === 0xa1) cursor += 1;
let unsignedAttributeOids: string[] | null = null;
const unsigned = children[cursor];
if (unsigned?.tag === 0xa1) {
unsignedAttributeOids = unsigned.children.map((attribute) => {
const fields = sequence(attribute, "SignerInfo unsigned attribute");
const oid = fields[0];
if (fields.length !== 2 || oid?.tag !== 0x06 || fields[1]?.tag !== 0x31)
return fail(
"SignerInfo unsigned attribute is missing its type or values.",
);
// OID text expands into decimal arcs and is retained in the result.
chargeDecodedName(names, oid.content.length * 4);
return decodeOid(oid.content);
});
cursor += 1;
}
if (cursor !== children.length)
return fail("SignerInfo has unexpected trailing fields.");
return {
@@ -527,6 +543,7 @@ const parseSigner = (
serial_hex,
subject_key_identifier_hex,
authenticated_attributes: authenticated,
unsigned_attribute_oids: unsignedAttributeOids,
};
};
+34 -1
View File
@@ -21,7 +21,10 @@ import {
PeCertificateAsn1Failure,
readAsn1,
} from "./PeCertificateAsn1.js";
import { parsePeCertificates } from "./PeCertificateParser.js";
import {
parsePeCertificates,
UNSIGNED_ATTRIBUTES_LIMITATION,
} from "./PeCertificateParser.js";
const defaults = inspectPeCertificatesInputSchema.parse({
path: "/artifacts/app.exe",
@@ -72,6 +75,36 @@ it("reads PE32 and PE32+ PKCS#7 certificate tables", () => {
}
});
it("reports unsigned attribute OIDs and incomplete nested coverage", () => {
const types = [
[1, 3, 6, 1, 4, 1, 311, 3, 3, 1],
[1, 3, 6, 1, 4, 1, 311, 2, 4, 1],
];
for (const plus of [false, true]) {
const report = peCertificatesSchema.parse(
parse(
peCertificateFixture(
plus,
pkcs7SignedDataFixture(undefined, undefined, undefined, types),
),
),
);
expect(
report.entries[0]?.pkcs7?.signers[0]?.unsigned_attribute_oids,
).toEqual(["1.3.6.1.4.1.311.3.3.1", "1.3.6.1.4.1.311.2.4.1"]);
expect(report.coverage).toMatchObject({
status: "partial",
certificates: 1,
});
expect(report.limitations).toContain(UNSIGNED_ATTRIBUTES_LIMITATION);
const control = parse(peCertificateFixture(plus));
expect(
control.entries[0]?.pkcs7?.signers[0]?.unsigned_attribute_oids,
).toBeNull();
expect(control.coverage.status).toBe("complete");
}
});
it("accepts only zero eight-byte alignment padding included in dwLength", () => {
for (const plus of [false, true]) {
const bytes = peCertificateFixture(plus, undefined, undefined, true);
+17 -4
View File
@@ -43,6 +43,9 @@ const LIMITATIONS = [
const TELETEX_LIMITATION =
"TeletexString values use OpenSSL's ISO-8859-1 compatibility mapping; this is not a full T.61 decoder.";
export const UNSIGNED_ATTRIBUTES_LIMITATION =
"Signer unsigned attribute contents are not decoded; nested signatures and timestamp token certificates and times may be omitted.";
const ABSENT_LIMITATIONS = [
"IMAGE_DIRECTORY_ENTRY_SECURITY is absent. That is not proof the image is unsigned in every catalog or overlay sense.",
"This workflow does not verify signatures, certificate chains, timestamps, or Authenticode digest coverage of image bytes.",
@@ -82,6 +85,7 @@ export const parsePeCertificates = (
let at = 0;
let examined = 0;
let teletexStringCompatibilityUsed = false;
let unsignedAttributesPresent = false;
while (at < table.length) {
// Alignment padding sits inside the gap before the next 8-byte boundary.
// Bytes that remain after that boundary are a short header, including
@@ -123,6 +127,9 @@ export const parsePeCertificates = (
const parsed = parsePkcs7SignedData(root);
teletexStringCompatibilityUsed ||=
parsed.teletexStringCompatibilityUsed;
unsignedAttributesPresent ||= parsed.signers.some(
(signer) => signer.unsigned_attribute_oids !== null,
);
pkcs7 = {
content_type_oid: parsed.content_type_oid,
content_type_name: oidName(parsed.content_type_oid),
@@ -166,6 +173,10 @@ export const parsePeCertificates = (
serial_hex: signer.serial_hex,
subject_key_identifier_hex: signer.subject_key_identifier_hex,
authenticated_attributes: signer.authenticated_attributes,
unsigned_attribute_oids:
signer.unsigned_attribute_oids === null
? null
: [...signer.unsigned_attribute_oids],
})),
};
} catch (cause) {
@@ -211,12 +222,14 @@ export const parsePeCertificates = (
},
entries,
coverage: {
status: "complete",
status: unsignedAttributesPresent ? "partial" : "complete",
examined_entries: examined,
certificates,
},
limitations: teletexStringCompatibilityUsed
? [...LIMITATIONS, TELETEX_LIMITATION]
: [...LIMITATIONS],
limitations: [
...LIMITATIONS,
...(teletexStringCompatibilityUsed ? [TELETEX_LIMITATION] : []),
...(unsignedAttributesPresent ? [UNSIGNED_ATTRIBUTES_LIMITATION] : []),
],
};
};
+13
View File
@@ -67,6 +67,7 @@ export const pkcs7SignedDataFixture = (
commonName = "REA Fixture",
organization = "REA Tests",
signature = octet(Buffer.from("sig")),
unsignedAttributes?: readonly (readonly number[])[],
): Buffer => {
const serial = Buffer.from([0x11, 0x22, 0x33, 0x44]);
const subject = name(commonName, organization);
@@ -94,6 +95,18 @@ export const pkcs7SignedDataFixture = (
algorithm(SHA256),
algorithm(SHA256_RSA),
signature,
...(unsignedAttributes === undefined
? []
: [
context(
1,
concat(
...unsignedAttributes.map((type) =>
seq(oid(type), set(octet(Buffer.from([1])))),
),
),
),
]),
);
const signedData = seq(
integer(1),