Repository navigation
[LI-197038] Prevent GraphQL injection in transfer method configuration queries - #155
Closed
brsouzapaypal wants to merge 1 commit into
Closed
brsouzapaypal wants to merge 1 commit into
brsouzapaypal wants to merge 1 commit into
Conversation
…n queries Caller-supplied values were interpolated directly into GraphQL query strings via String(format:), allowing a crafted value to break out of its field and append arbitrary GraphQL (CWE-943). Add two helpers on the GraphQlQuery protocol and wire them into every query that interpolates caller input: - sanitizeGraphQlLiteral: allowlist of GraphQL-name characters for unquoted enum/scalar literals (country, currency, profile, transferMethodType) - escapeGraphQlString: escape backslashes and double quotes for values embedded inside quoted strings (user and transfer method tokens) Legitimate values pass through unchanged; injection payloads lose the syntax characters they rely on. Adds unit tests covering injection neutralization, quote escaping, and preservation of valid values.
Author
|
Closing this PR — no longer needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a GraphQL injection vulnerability (CWE-943, LI-197038) in the transfer method configuration queries. Caller-supplied values were interpolated directly into the GraphQL query string via
String(format:), so a crafted value could terminate its field and append arbitrary GraphQL (e.g. an introspection selection).Changes
GraphQlQuery: new protocol-extension helperssanitizeGraphQlLiteral(_:): allowlist of GraphQL-name characters for values interpolated as unquoted enum/scalar literals.escapeGraphQlString(_:): escapes\and"for values embedded inside quoted strings.HyperwalletTransferMethodConfigurationFieldQuery(userToken, profile, country, currency, transferMethodType)HyperwalletTransferMethodConfigurationKeysQuery(userToken)HyperwalletTransferMethodTypesFeesAndProcessingTimesQuery(userToken, country, currency)HyperwalletTransferMethodUpdateConfigurationFieldQuery(transferMethodToken)Scope note
The ticket enumerates the field query fields. The fix was intentionally broadened to every query that interpolates caller input (KeysQuery, FeesAndProcessingTimes, and all
userTokenvalues), since the same unsafe pattern repeats there.Testing
HyperwalletTransferMethodConfigurationFieldQueryTestscovering injection neutralization in unquoted literals, quote escaping in quoted strings, and preservation of valid values for all four queries.CI note
The
iOS Core SDK CIworkflow fails at theCarthage [Install dependencies]step while building theHippolytetest dependency under Xcode 16.2 (Build Failed, exit code 70), before the project is compiled or the test/lint steps run. This is a pre-existing pipeline failure unrelated to this change.How this was verified:
iOS Core SDK CIworkflow against this branch.Carthage [Install dependencies]step; theRun(tests) andLint validationsteps never executed.masterCI run, which fails at the same early stage, confirming the failure predates this change.Because the dependency build fails before the test phase, the test and lint steps cannot execute in CI until the
Hippolytebuild is repaired. The added XCTest cases will run once the pipeline is healthy. In the meantime the change was validated locally.