Skip to content

Add a failable initializer for parsing untrusted OID strings - #121

Open
Nadav0077 wants to merge 5 commits into
apple:mainfrom
Nadav0077:add-failable-oid-string-init
Open

Add a failable initializer for parsing untrusted OID strings#121
Nadav0077 wants to merge 5 commits into
apple:mainfrom
Nadav0077:add-failable-oid-string-init

Conversation

@Nadav0077

@Nadav0077 Nadav0077 commented Apr 30, 2026

Copy link
Copy Markdown

Motivation:

ASN1ObjectIdentifier.init(dotRepresentation:) is documented as a parser for already-validated input and used to terminate the process when the components did not satisfy the ASN.1 OID rules from ITU-T X.690 8.19.4 or when the encoding step
(firstComponent * 40) + secondComponent overflowed UInt. Callers that need to accept OID strings from untrusted sources currently have no built-in way to reject malformed input without risking a process abort.

Modifications:

  • Add ASN1ObjectIdentifier.init?(validating: String) alongside the existing throwing variant. The new initializer validates the string syntax, validates the first arc (must be 0, 1, or 2) and the second arc (must be less than 40 when the first is 0 or 1), and uses addingReportingOverflow for the residual case where the first arc is 2 and the second arc is close to UInt.max. Any failure returns nil.
  • Rewrite the existing throwing dot-representation initializers to delegate through the validating initializer, keeping the parsing and encoding logic shared.
  • Add a new test, testOIDFailableStringInitializer, covering the happy path and every rejection case (malformed input, out-of-range first/second arcs, both overflow paths).

Result:

Callers can now parse OID strings from untrusted sources by writing ASN1ObjectIdentifier(validating: input) and treating a nil result as a parse failure, with no risk of terminating the process.

Motivation:

`ASN1ObjectIdentifier.init(dotRepresentation:)` is documented as a
parser for already-validated input and terminates the process when
the components do not satisfy the ASN.1 OID rules from ITU-T X.690
§8.19.4 or when the encoding step
`(firstComponent * 40) + secondComponent` overflows `UInt`. Callers
that need to accept OID strings from untrusted sources currently have
no built-in way to reject malformed input without risking a process
abort.

Modifications:

* Add `ASN1ObjectIdentifier.init?(validating: String)` alongside the
  existing throwing variant. The new initializer performs the same
  syntactic parse and additionally validates the first arc (must be
  0, 1, or 2) and the second arc (must be less than 40 when the first
  is 0 or 1), and uses `addingReportingOverflow` for the residual
  case where the first arc is 2 and the second arc is close to
  `UInt.max`. Any failure returns `nil`.
* Existing throwing initializers and array-literal conformance are
  left unchanged.
* Add a new test, `testOIDFailableStringInitializer`, covering the
  happy path and every rejection case (malformed input, out-of-range
  first/second arcs, both overflow paths).

Result:

Callers can now parse OID strings from untrusted sources by writing
`ASN1ObjectIdentifier(validating: input)` and treating a `nil`
result as a parse failure, with no risk of terminating the process.
@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Jun 22, 2026
Comment thread Sources/SwiftASN1/Basic ASN1 Types/ObjectIdentifier.swift Outdated
@Nadav0077
Nadav0077 requested a review from Lukasa June 22, 2026 12:52

@Lukasa Lukasa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you push the change?

@Nadav0077

Copy link
Copy Markdown
Author

@Lukasa Yep, pushed as 9f1c120. The PR head is now that commit.

@Nadav0077
Nadav0077 requested a review from Lukasa June 23, 2026 06:09

@Lukasa Lukasa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm still not seeing anything in the diff actually replacing the body of the other initializer

@Nadav0077

Copy link
Copy Markdown
Author

@Lukasa Sorry, this should be fixed now. I also cleaned up the docs/PR description so they match the new delegation path.

@Nadav0077
Nadav0077 requested a review from Lukasa June 23, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants