Skip to content

x509asn1: make encodeOID stop on too long input - #20871

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/x509-fixes
Closed

bagder wants to merge 1 commit into
masterfrom
bagder/x509-fixes

Conversation

@bagder

@bagder bagder commented Mar 9, 2026

Copy link
Copy Markdown
Member

Plus two other minor fixups.

Reported-by: John Rodriguez

@bagder bagder added the TLS label Mar 9, 2026
@bagder

bagder commented Mar 9, 2026

Copy link
Copy Markdown
Member Author

@aisle-analyzer augment review

@aisle-research-bot

aisle-research-bot Bot commented Mar 9, 2026 •

Copy link
Copy Markdown

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

Aisle supplements but does not replace security review.


Analyzed PR: #20871 at commit a0f4ebb

Last updated on: 2026-03-09T19:31:41Z

@bagder
bagder marked this pull request as ready for review March 9, 2026 19:19
@augmentcode

augmentcode Bot commented Mar 9, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Tightens X.509 ASN.1 parsing to avoid overruns/underflows on malformed inputs.
Changes: encodeOID() now bails out when OID subidentifiers run past the input, Curl_parseX509() avoids calling getASN1Element() with a NULL cursor, and ECC public key length computation adds a minimum-size guard.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/vtls/x509asn1.c
Comment thread lib/vtls/x509asn1.c Outdated
bagder added a commit that referenced this pull request Mar 9, 2026
Plus two other minor fixups.

Reported-by: John Rodriguez

Closes #20871
@bagder
bagder force-pushed the bagder/x509-fixes branch from 47a512d to 5962ed2 Compare March 9, 2026 20:42
@bagder
bagder requested a review from Copilot March 9, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens X.509 ASN.1 parsing and certificate info extraction by improving bounds/error handling in OID parsing and public key parsing, plus a small cleanup around dynbuf lifetime.

Changes:

  • Prevent out-of-bounds reads in encodeOID() when encountering a truncated base-128 OID component.
  • Ensure temporary dynbuf used for symbolic OID lookup is always freed.
  • Add additional guardrails in X.509 parsing (issuer extraction) and ECC public key handling for undersized inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/vtls/x509asn1.c Outdated
Comment thread lib/vtls/x509asn1.c Outdated
bagder added a commit that referenced this pull request Mar 9, 2026
Plus two other minor fixups.

Reported-by: John Rodriguez

Closes #20871
@bagder
bagder force-pushed the bagder/x509-fixes branch from 5962ed2 to dba183e Compare March 9, 2026 20:48
Plus two other minor fixups.

Reported-by: John Rodriguez

Closes #20871
@bagder
bagder force-pushed the bagder/x509-fixes branch from dba183e to f8f85b4 Compare March 9, 2026 20:53
@bagder bagder closed this in 248dd9e Mar 9, 2026
@bagder
bagder deleted the bagder/x509-fixes branch March 9, 2026 21:07
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Plus a minor fixup.

Reported-by: John Rodriguez

Closes curl#20871
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants