OWASP / OWASP/cve-lite-cli

fix: isSslCertificateError skips the outer message whenever the error has a cause

Open
#1,147 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug in-house
Dominant language
TypeScript
Stars
715
Forks
145
Avg merge
21h 39m
Merged PRs (30d)
66

Description

Note: this is an in-house item already being handled by the maintainer - not open for contribution. Filed for tracking only.

isSslCertificateError in src/utils/network.ts:101 recurses into cause as soon as one exists, and never falls back to checking the outer error's own message:

export function isSslCertificateError(error: unknown): boolean {
  if (!(error instanceof Error)) return false;
  const code = (error as NodeJS.ErrnoException).code;
  if (code && SSL_ERROR_CODES.has(code)) return true;
  const cause = (error as { cause?: unknown }).cause;
  if (cause) return isSslCertificateError(cause);          // <- returns, never falls through
  const normalized = error.message.toLowerCase();
  return SSL_ERROR_MESSAGE_FRAGMENTS.some(f => normalized.includes(f));
}

So an error whose own message names a certificate problem, but which carries any unrelated cause, is not recognised as an SSL error. The message-fragment check at the end is unreachable for every error that has a cause at all.

Concretely, this returns false today:

new Error("self signed certificate in certificate chain", { cause: new Error("socket hang up") })

The same error with no cause returns true.

The practical effect is that the user gets the generic fallback hint instead of sslCertificateErrorHint(), which is the one that actually tells them to run cve-lite config set ca-cert. That is the wrong hint for someone sitting behind a corporate proxy, and it is the exact audience the feature was built for in #395.

The fix is to check the outer message before, or as well as, recursing, rather than returning the recursive result unconditionally. Worth deciding whether a match anywhere in the chain should win, which is probably what was intended.

Found while reviewing #1137, which touches the same function but is not the cause of this. Pre-existing on main.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at src/utils/network.ts:101 and inspect isSslCertificateError, using the provided error with a certificate message and unrelated cause as the reproduction. Done means certificate errors are recognized when the outer message matches, including when a cause exists, so sslCertificateErrorHint() is selected instead of the generic fallback.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
cli, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.