Conversation
Signed-off-by: Ankit Basu <ankitbasu14@gmail.com>
| var signatures [][]byte | ||
| var inputTokens []*snarktoken.Input | ||
|
|
||
| for i, inputID := range inputIDs { |
There was a problem hiding this comment.
The validation chain (TransferSignatureValidate + TransferZKValidate) never checks that the SNARK-proven Inputs[i].CommitmentIn corresponds to the token identified by the new InputIDs[i]/InputTokens[i] fields. Authorization and ledger deletion are decoupled from what the proof actually spends.
Failure scenario: An attacker owning tokens A (value 100) and B (value 1) builds a ZK-valid transfer whose SpendDescription proves the opening of A, but sets InputIDs/InputTokens to reference B. TransferSignatureValidate loads B from the ledger, finds the attacker as owner, and accepts the signature; TransferZKValidate verifies the (valid) proof over A's commitment; the RWS translator deletes B (value 1). Value conservation is enforced only over A (value 100), so the attacker mints ~99 units. Neither validator checks that Inputs[i].CommitmentIn == OutputDescription(InputTokens[i]).CommitmentOut
|
|
||
| for i, inputID := range inputIDs { | ||
| // Load the on-chain output (OutputDescription) to get the owner | ||
| raw, err := ctx.Ledger.GetState(*inputID) |
There was a problem hiding this comment.
A crafted TokenRequest with a transfer action JSON
{"InputIDs":[null],"Inputs":[{...one spend...}]}deserializes to a []*token2.ID slice containing a nil pointer.
The len(inputIDs)==len(action.Inputs) guard passes (both 1), then ctx.Ledger.GetState(*inputID) dereferences the nil pointer and panics, crashing the validating node on untrusted input.
| action.InputIDs = tokenIDs | ||
| var inputTokens [][]byte | ||
| for _, loadedToken := range loadedTokens { | ||
| inputTokens = append(inputTokens, loadedToken.Token) |
There was a problem hiding this comment.
nit: append in a loop -> best to pre-allocate and index assign
Something like this:
inputTokens := make([][]byte, len(loadedTokens))
for i, token := range loadedTokens {
inputTokens[i] = token.Token
}| } | ||
|
|
||
| var outputDesc snarktoken.OutputDescription | ||
| if err := json.Unmarshal(raw, &outputDesc); err != nil { |
There was a problem hiding this comment.
This PR adds new fields to TransferAction.Deserialize (InputIDs/InputTokens/Issuer) and adds a validator that json.Unmarshals ledger tokens, but adds no fuzz target and no {name,pkg,func} entry to .github/workflows/nightly-fuzz.yml
The new parsing surface is never exercised under -fuzztime.
| return a.InputIDs | ||
| } | ||
|
|
||
| // GetSerializedInputs returns the serialized inputs of the action. |
There was a problem hiding this comment.
GetSerializedInputs falls back to serializing SpendDescriptions when InputTokens is empty, producing OutputSN keys that can never match the on-ledger token keys.
Failure scenario: If a TransferAction has InputIDs set but InputTokens empty (an action built/reconstructed without the new population step, or a wire action omitting InputTokens), GetSerializedInputs returns marshaled SpendDescriptions. translator.checkInputs then computes CreateOutputSNKey(TxId, Index, <SpendDescription bytes>), which never equals the key stored from the token's OutputDescription, so StateMustExist fails with 'input must exist'...
A hard-to-diagnose failure driven by which field happens to be populated.
| transferRaw, err := transferAction.Serialize() | ||
| require.NoError(t, err) | ||
|
|
||
| tr := &driver.TokenRequest{ |
There was a problem hiding this comment.
Signatures here are never used.
so the test does not exercise the common framework's signature-scoping/EnsureExhausted path it claims to regression-test.
2 participants
Summary
This PR adds an end-to-end regression test suite for the
zkatsnarktoken driver and resolves key transfer action validation and signature verification requirements:regression_test.go):IssueandTransfer) end-to-end using real SNARK provers (SpendProver,OutputProver), circuits, and validators.validator_transfer_sig.go):TransferSignatureValidateto satisfy the common token framework validation contract.EnsureExhausted).TransferActionto hold and serialize original inputOutputDescriptions inInputTokens, ensuring ledger lookup and token key verification succeed during transfer validation.Testing
All tests pass