search

LokartBC / ZKBattleship_secure

Creative Commons Attribution Share Alike 4.0 International
0 stars 0 forks source link

missing validation on private input signals #1

Open lhoste-bell opened 2 years ago

lhoste-bell commented 2 years ago

Hi @LokartBC,

Great job on bringing multiple improvements to the original zkbattleship-circuit.

I would suggest to add additional input validation on the private input signals. Currently, the prover is able to modify private positions of the ships during gameplay.

For example on https://github.com/LokartBC/ZKBattleship_secure/blob/a37ec16d1e57539e2469fc45cc28eead17740bfd/circuits/battleShipHit.circom#L24 one can increment the carrier signal while decrementing the cruiser signal. The hash will remain the same, but the location of the ships are modified:

--- a/circuits/inputs/battleShipHit_packed.json
+++ b/circuits/inputs/battleShipHit_packed.json
@@ -1,10 +1,10 @@
 {
-"carrier" : 1100586419201,
+"carrier" : 1100586419202,
 "battleship" : 2149582850,
-"cruiser" : 4198404,
+"cruiser" : 4198403,
 "submarine" : 8396808,
 "destroyer" : 16400,
 "salt" : 11111,
 "publicShipHash": "3259213925179050331258828360586916704077545552103193146303509714212511170244",
 "target": 1
}

fyi, a similar problem exists in the original codebase as well: https://github.com/tommymsz006/zkbattleship-circuit/blob/24d6d95e674176b72d89babd82d194c72786851b/circom/battleship_pedersen.circom#L35

LokartBC commented 2 years ago

Hi @lhoste-bell, you are perfectly right, thank you very much. I will adapt the code to correct this !