Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions openid-4-verifiable-presentations-1_0.md
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,10 @@ these constraints. The Wallet is not controlled by the Verifier and the Verifier
MUST perform its own security checks on the returned Credentials and
Presentations.

## Security Checks on the DCQL Query {#dcql_injection}

Recipients of DCQL query MUST treat the incoming query as untrusted input. A malformed or malicious statement could trigger a "DCQL injection" altering the normal processing. Implementations MUST NOT rely on the Verifier for sanitation; they MUST independently validate query syntax, semantics, permitted scopes, resource identifiers, and execution context before acceptance or evaluation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks Simone. I'm not sure this is actually the most actionable advice though - if the verifier is allowing the user to modify the DCQL query without proper escaping, then there will almost certainly be ways the attacker can modify the query such that is it still valid.

The advice around preventing SQL injection (e.g. https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html ) is all around the component preparing the SQL query doing it correctly, so I think the primary thing we need to say is that verifiers need to ensure proper escaping if they are including user-sourced input into the DCQL query.

I think this should really only be problematic in the case where value matching is being used, I'm not sure there's anywhere else in the DCQL query where there would be a reason to include user provided content.

Copy link
Author

Choose a reason for hiding this comment

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

@jogu you're welcome.

Using the example of SQL injection, one of the most famous cases, the best solution is to use prepared statements (i.e., a parametric query). I'm not sure how applicable this is to the specific case.

The point is that in SQL injection, we typically have: User (malicious) > modifies the input > application server that performs the query > database that receives it.

Here we have: Verifier (malicious) > User > the query > Wallet.

The threat source is from those who make a query, but the impact is felt first and foremost by those who receive and process it.

Current security considerations rightly advise that you should not trust what the Verifier receives, and the concept remains largely the same. So it makes sense to tell the recipient of the query not to trust it and to verify it.

Therefore, checking the query structure at the format level (JSON) could be useful (to prevent someone from changing the semantics of that query), and this can be done in several ways. Validating the JSON and checking for characters that have semantic value.

Concerning the use case, I was conducting some experiments via digitalcredentials.dev (and was preparing a Proof of Concept, but I was advised to write first due to time constraints).

Copy link
Collaborator

Choose a reason for hiding this comment

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

So I'm not sure SQL injection is the right parallel for the problem you're trying to describe, as in the SQL injection case the attacker is trying to manipulate the site under attack to produce SQL that doesn't do what the site originally intended. That kind of manipulation is potentially also applicable to OID4VP verifiers, but only if they include user input into the query and don't generate the JSON correctly.

Verifier (malicious) > User > the query > Wallet.

I don't think the "user" is really a player at that point in the flow (unless they're acting as an attacker, and if they are the right way to mitigate that threat is signed requests).

Therefore, checking the query structure at the format level (JSON) could be useful (to prevent someone from changing the semantics of that query)

I mean perhaps, but really there is a single component that is consuming the query, so the semantics are what they are.

, and this can be done in several ways. Validating the JSON

Calling out explicitly that invalid JSON needs to be rejected seems fine, I'd imagine most wallets are already doing that

checking for characters that have semantic value.

I don't really understand what this means a wallet needs to do. Maybe there's something that could be done but it's heuristic perhaps - there's not really any characters that have semantic value after you've parsed the json, and at parsing point the json is either invalid or valid (and obviously you should reject invalid json).


# Privacy Considerations {#privacy-considerations}

Many privacy considerations are specific to the Credential format and associated proof type used in a particular Presentation.
Expand Down