Closed wadeking98 closed 6 months ago
Colons should be escaped in the tag since it is user controlled input.
I would suggest that requirements for handling this include:
Rather than escaping the colons, the easiest and fastest solution is simply to explicitly prohibit colons in all parts of the colon-separated IDs and let the implementer determine whether or not and how the schema names, tags, etc. need to be escaped.
For example, to use a URL as a schema name, I used urllib.parse.quote(...)
, resulting in this schema ID:
FxLrCGaSTrVjjfwA7t2Ma9:2:http%3A//Ennovate.com/schemas/person.yaml:1.0
entry in ledger as of 08/25/2023
BTW "defenition" in issue title should be "definition".
TLDR:
The user creating a credential definition can use colons in the tag to make indy-sdk interpret the credential definition differently. While I haven't been able to successfully exploit this, but I don't think the user should have this much control over how the credential definition is interpreted. Colons should be escaped in the tag since it is user controlled input.
The Long Version:
I first came across this issue a few months ago exterminating in AFJ, I have been able to reproduce it on acapy as well. I noticed that if I added anything with colons as the tag for a credential definition, then the credential definition will be interpreted differently. For example, if I use the tag
test:test
then I get the following error when I try to make a new credDef:I did some digging into indy-sdk and found the point where it was failing in
So if I change the tag to
credential_definition.rs
. The reason it's failing is because there's no 6-part credDef format, however there is a 7,8,9,16 part credDef format:test:test:test:test:test
then that makes a 9 part credential definition and I get the following error:If I can change the signature type (CL) I might be able to convince
CredentialDefinitionId
that this credential definition is coming from a different DID. I don't think this is a security issue because the signatures wouldn't match, but I still think it's a priority to patch.Steps to reproduce:
test:test:test:test:test