fix(webauthn): Fix buffer out of bounds

[satoshiotomakan]

This pull request improves the robustness and error handling of the WebAuthn implementation, particularly in parsing and extracting public keys and ECDSA signature values. The main changes include making parsing functions more resilient to malformed input and updating interfaces and tests to handle possible failures gracefully.

Breaking Changes

• Changed the return type of TWWebAuthnGetRSValues from non-nullable to nullable, and updated its implementation to return nullptr if parsing fails. This ensures that invalid signatures are handled safely.

Error handling and robustness improvements:

• Updated the parseAuthData function to return std::optional<AuthData> and added checks for minimum buffer sizes, preventing out-of-bounds access when parsing authentication data.
• Improved the getPublicKey function to use exception handling and additional checks for missing or malformed data, returning std::nullopt on failure.

Test and interface updates:

• Updated Swift and C++ tests to handle nullable return values from getRSValues, ensuring tests reflect the new error handling behavior.

[Copilot]

Pull request overview

This PR enhances the robustness of the WebAuthn implementation by adding proper error handling and buffer bounds checking to prevent out-of-bounds memory access. The main change is making TWWebAuthnGetRSValues return nullable to handle parsing failures gracefully.

Key Changes:

• Made TWWebAuthnGetRSValues return _Nullable instead of _Nonnull to handle malformed signatures
• Added buffer size validation in parseAuthData to prevent out-of-bounds access
• Improved getPublicKey error handling with try-catch and additional validation checks

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
include/TrustWalletCore/TWWebAuthn.h	Changed return type of TWWebAuthnGetRSValues to nullable
src/interface/TWWebAuthn.cpp	Updated implementation to check optional and return nullptr on failure
src/WebAuthn.cpp	Added buffer size constants and validation checks in parseAuthData and getPublicKey
tests/common/WebAuthnTests.cpp	Updated test to use pointer instead of reference for nullable return
swift/Tests/WebAuthnTests.swift	Added force unwrap operator to handle nullable return in test

Comments suppressed due to low confidence (1)

tests/common/WebAuthnTests.cpp:37

• The test only covers the success case for TWWebAuthnGetRSValues, but the PR introduces error handling for malformed signatures. Consider adding a test case for invalid/malformed signatures to verify that the function correctly returns nullptr when parsing fails.

Example:

// Test with invalid signature
const auto invalidSignature = DATA("0x1234"); // Too short/malformed
const auto* rsValuesData = TWWebAuthnGetRSValues(invalidSignature.get());
EXPECT_EQ(rsValuesData, nullptr);

TEST(WebAuthn, GetRSValues) {

    // C
    {
        const auto signature = DATA("0x30440220766589b461a838748708cdf88444b21b1fa52b57d70671b4f9bf60ad14b372ec022020cc439c9c20661bfa39bbea24a900ec1484b2395eb065ead8ef4e273144a57d");
        const auto* rsValuesData = TWWebAuthnGetRSValues(signature.get());
        const auto& rsValues = hexEncoded(*reinterpret_cast<const Data*>(WRAPD(rsValuesData).get()));
        EXPECT_EQ(rsValues, "0x766589b461a838748708cdf88444b21b1fa52b57d70671b4f9bf60ad14b372ec20cc439c9c20661bfa39bbea24a900ec1484b2395eb065ead8ef4e273144a57d");
    }
}

---

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

[github-actions]

Binary size comparison

➡️ aarch64-apple-ios: 14.31 MB

➡️ aarch64-apple-ios-sim: 14.31 MB

➡️ aarch64-linux-android: 18.87 MB

➡️ armv7-linux-androideabi: 15.80 MB

➡️ wasm32-unknown-emscripten: 13.46 MB

[gupnik]

Should we add a couple of tests that pass the malformed data and assert that the returns are nulls?

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.

---

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 81 out of 81 changed files in this pull request and generated 4 comments.

---

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.