X-Wing integrityCheckedRepresentation initializer should validate length like ML-DSA does
- Dominant language
- Assembly
- Stars
- 1.7k
- Forks
- 223
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 7
Description
### Summary
The `integrityCheckedRepresentation` initializer for X-Wing keys does not validate the length of the input representation before processing it, unlike the equivalent ML-DSA initializer which includes an explicit length guard. Both types follow the same pattern - reconstruct a key from a representation that includes an integrity-check suffix - but only ML-DSA validates the input length upfront.
### ML-DSA (has the guard - `MLDSA.swift`):
```swift
init(integrityCheckedRepresentation: some DataProtocol) throws {
guard integrityCheckedRepresentation.count == seedSize + 32 else {
throw CryptoKitError.incorrectParameterSize
}
// ... proceeds with dropLast(32) / dropFirst(32)
}
```
### X-Wing (missing the guard - `XWing.swift:121-127`):
```swift
init(integrityCheckedRepresentation: some DataProtocol) throws {
// No length check here
let seedRepresentation = integrityCheckedRepresentation.dropLast(32)
let hashSuffix = integrityCheckedRepresentation.suffix(32)
// ... proceeds to hash and compare
}
```
The X-Wing path is incidentally safe because `SHA3_256Digest(bytes:)` tolerates arbitrary-length input and the downstream seed initializer at line 178 validates `privateKeyBytes.count == XWING_PRIVATE_KEY_BYTES`. However, the missing guard is an inconsistency: if the downstream validation ever changes, the missing length check becomes a latent bug. More immediately, passing a wrong-length representation to the X-Wing initializer silently computes a hash comparison on incorrect data before eventually failing at the seed init, rather than failing fast with `incorrectParameterSize`.
### Proposed fix
Add the same length guard to match ML-DSA's pattern:
```swift
init(integrityCheckedRepresentation: some DataProtocol) throws {
guard integrityCheckedRepresentation.count == /* expected seed size */ + 32 else {
throw CryptoKitError.incorrectParameterSize
}
let seedRepresentation = integrityCheckedRepresentation.dropLast(32)
let hashSuffix = integrityCheckedRepresentation.suffix(32)
// ... rest unchanged
}
```
Replace `/* expected seed size */` with the appropriate constant (likely `XWING_PRIVATE_KEY_BYTES` or the corresponding seed byte count - verify against the X-Wing spec).
### Impact
Consistency / defense-in-depth. Not a vulnerability - the downstream seed init catches wrong-length input. Adding the upfront guard makes the X-Wing path fail fast with a clear error and matches the ML-DSA pattern, reducing the risk of a future regression.
### References
- The ML-DSA equivalent guard for comparison
- X-Wing spec (draft-connolly-cfrg-xwing-kem)
Contributor guide
Research direction
Start in XWing.swift:121-127 and compare the initializer with the guard in MLDSA.swift. Verify the expected representation length against XWing.swift:178 and the X-Wing specification, then add the upfront validation so incorrect-length inputs fail with incorrectParameterSize before hashing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- cryptography
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 74/100