Skip to content

fix(passport-signature): guard ECDSA bruteforce returns and add safe fallbacks #1386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −18
Open

fix(passport-signature): guard ECDSA bruteforce returns and add safe fallbacks #1386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions common/src/utils/passports/passport_parsing/brutForceDscSignature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import { hash } from '../../hash.js';
export function brutforceSignatureAlgorithmDsc(dsc: CertificateData, csca: CertificateData) {
if (csca.signatureAlgorithm === 'ecdsa') {
const hashAlgorithm = brutforceHashAlgorithmDsc(dsc, csca, 'ecdsa');
return {
signatureAlgorithm: 'ecdsa',
hashAlgorithm: hashAlgorithm,
saltLength: 0,
};
if (hashAlgorithm) {
return {
signatureAlgorithm: 'ecdsa',
hashAlgorithm: hashAlgorithm,
saltLength: 0,
};
}
} else if (csca.signatureAlgorithm === 'rsa') {
const hashAlgorithm = brutforceHashAlgorithmDsc(dsc, csca, 'rsa');
if (hashAlgorithm) {
Expand Down
12 changes: 7 additions & 5 deletions common/src/utils/passports/passport_parsing/brutForcePassportSignature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ export function brutforceSignatureAlgorithm(passportData: PassportData) {
const parsedDsc = parseCertificateSimple(passportData.dsc);
if (parsedDsc.signatureAlgorithm === 'ecdsa') {
const hashAlgorithm = brutforceHashAlgorithm(passportData, 'ecdsa');
return {
signatureAlgorithm: 'ecdsa',
hashAlgorithm: hashAlgorithm,
saltLength: 0,
};
if (hashAlgorithm) {
return {
signatureAlgorithm: 'ecdsa',
hashAlgorithm: hashAlgorithm,
saltLength: 0,
};
}
} else if (parsedDsc.signatureAlgorithm === 'rsa') {
const hashAlgorithm = brutforceHashAlgorithm(passportData, 'rsa');
if (hashAlgorithm) {
Expand Down
12 changes: 9 additions & 3 deletions common/src/utils/passports/passport_parsing/parseDscCertificateData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,17 @@ export function parseDscCertificateData(
cscaParsed = parseCertificateSimple(csca);
const details = brutforceSignatureAlgorithmDsc(dscCert, cscaParsed);
cscaFound = true;
cscaHashAlgorithm = details.hashAlgorithm;
cscaSignatureAlgorithm = details.signatureAlgorithm;
if (details && details.hashAlgorithm && details.signatureAlgorithm) {
cscaHashAlgorithm = details.hashAlgorithm;
cscaSignatureAlgorithm = details.signatureAlgorithm;
cscaSaltLength = details.saltLength;
} else if (cscaParsed) {
cscaHashAlgorithm = cscaParsed.hashAlgorithm;
cscaSignatureAlgorithm = cscaParsed.signatureAlgorithm;
cscaSaltLength = 0;
}
Comment on lines +38 to +46
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard against undefined cscaSaltLength in return object.

The fallback logic doesn't cover all cases. If both conditions fail (no valid details and no cscaParsed), cscaSaltLength remains undefined, but the interface DscCertificateMetaData expects it to be a number (line 13). This creates a type safety issue.

Apply this diff to ensure a safe default:

         if (details && details.hashAlgorithm && details.signatureAlgorithm) {
           cscaHashAlgorithm = details.hashAlgorithm;
           cscaSignatureAlgorithm = details.signatureAlgorithm;
           cscaSaltLength = details.saltLength;
         } else if (cscaParsed) {
           cscaHashAlgorithm = cscaParsed.hashAlgorithm;
           cscaSignatureAlgorithm = cscaParsed.signatureAlgorithm;
           cscaSaltLength = 0;
+        } else {
+          // Fallback to safe defaults if no valid data is available
+          cscaSaltLength = 0;
         }

Alternatively, update the interface to allow undefined values if that's the intended behavior.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (details && details.hashAlgorithm && details.signatureAlgorithm) {
cscaHashAlgorithm = details.hashAlgorithm;
cscaSignatureAlgorithm = details.signatureAlgorithm;
cscaSaltLength = details.saltLength;
} else if (cscaParsed) {
cscaHashAlgorithm = cscaParsed.hashAlgorithm;
cscaSignatureAlgorithm = cscaParsed.signatureAlgorithm;
cscaSaltLength = 0;
}
if (details && details.hashAlgorithm && details.signatureAlgorithm) {
cscaHashAlgorithm = details.hashAlgorithm;
cscaSignatureAlgorithm = details.signatureAlgorithm;
cscaSaltLength = details.saltLength;
} else if (cscaParsed) {
cscaHashAlgorithm = cscaParsed.hashAlgorithm;
cscaSignatureAlgorithm = cscaParsed.signatureAlgorithm;
cscaSaltLength = 0;
} else {
// Fallback to safe defaults if no valid data is available
cscaSaltLength = 0;
}
🤖 Prompt for AI Agents 
common/src/utils/passports/passport_parsing/parseDscCertificateData.ts around
lines 38-46: cscaSaltLength can remain undefined if neither details nor
cscaParsed is present, violating the DscCertificateMetaData number type; ensure
a safe numeric default by initializing cscaSaltLength to 0 when you declare it
or add an explicit final else that sets cscaSaltLength = 0 before the return (or
alternatively change the DscCertificateMetaData type to allow undefined if that
is intended).

cscaCurveOrExponent = getCurveOrExponent(cscaParsed);
cscaSignatureAlgorithmBits = parseInt(cscaParsed.publicKeyDetails.bits);
cscaSaltLength = details.saltLength;
}
} catch (error) {}
} else {
Expand Down
22 changes: 17 additions & 5 deletions common/src/utils/passports/passport_parsing/parsePassportData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,6 @@ export function parsePassportData(
passportData.signedAttr
);

const brutForcedPublicKeyDetails = brutforceSignatureAlgorithm(passportData);

let parsedDsc = null;
let dscSignatureAlgorithmBits = 0;

Expand All @@ -125,6 +123,20 @@ export function parsePassportData(
dscMetaData = parseDscCertificateData(parsedDsc, skiPem);
}

// Determine signature algorithm/hash with fallback if brute-force fails
const brutForcedPublicKeyDetails = brutforceSignatureAlgorithm(passportData);
let resolvedSignatureAlgorithm = brutForcedPublicKeyDetails?.signatureAlgorithm as string | undefined;
let resolvedSignedAttrHashFunction = brutForcedPublicKeyDetails?.hashAlgorithm as string | undefined;
let resolvedSaltLength = (brutForcedPublicKeyDetails?.saltLength as number | undefined) ?? undefined;

if (!resolvedSignatureAlgorithm || !resolvedSignedAttrHashFunction) {
if (parsedDsc) {
resolvedSignatureAlgorithm = parsedDsc.signatureAlgorithm;
resolvedSignedAttrHashFunction = parsedDsc.hashAlgorithm;
resolvedSaltLength = 0;
}
}

return {
dataGroups:
passportData.dgPresents
Expand All @@ -141,9 +153,9 @@ export function parsePassportData(
eContentHashFunction,
eContentHashOffset,
signedAttrSize: passportData.signedAttr?.length || 0,
signedAttrHashFunction: brutForcedPublicKeyDetails.hashAlgorithm,
signatureAlgorithm: brutForcedPublicKeyDetails.signatureAlgorithm,
saltLength: brutForcedPublicKeyDetails.saltLength,
signedAttrHashFunction: resolvedSignedAttrHashFunction,
signatureAlgorithm: resolvedSignatureAlgorithm,
saltLength: resolvedSaltLength || 0,
Comment on lines +156 to +158
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add fallbacks for signedAttrHashFunction and signatureAlgorithm to prevent undefined values.

While saltLength has a fallback to 0 (line 158), signedAttrHashFunction (line 156) and signatureAlgorithm (line 157) can be undefined if both brute-force and parsedDsc fail. The interface PassportMetadata likely expects these to be strings, creating a type mismatch.

Apply this diff to add safe fallbacks:

     signedAttrSize: passportData.signedAttr?.length || 0,
-    signedAttrHashFunction: resolvedSignedAttrHashFunction,
-    signatureAlgorithm: resolvedSignatureAlgorithm,
+    signedAttrHashFunction: resolvedSignedAttrHashFunction || 'unknown',
+    signatureAlgorithm: resolvedSignatureAlgorithm || 'unknown',
     saltLength: resolvedSaltLength || 0,

This aligns with the existing pattern of using 'unknown' as a fallback (e.g., line 159).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In common/src/utils/passports/passport_parsing/parsePassportData.ts around lines
156 to 158, add safe fallbacks for signedAttrHashFunction and signatureAlgorithm
to avoid undefined values: if resolvedSignedAttrHashFunction or
resolvedSignatureAlgorithm are undefined, default them to the string 'unknown'
(similar to the existing pattern used for other fields) while keeping saltLength
defaulting to 0.

curveOrExponent: parsedDsc ? getCurveOrExponent(parsedDsc) : 'unknown',
signatureAlgorithmBits: dscSignatureAlgorithmBits,
countryCode: passportData.mrz ? getCountryCodeFromMrz(passportData.mrz) : 'unknown',
Expand Down