Malleability of unsigned lock witnesses #10

XuJiandong · 2024-08-15T08:11:32Z

The index of the deposit block header hash in header_deps MUST be put in the type-script-part of the corresponding witness at index i, using 64-bit unsigned little-endian integer format. The example below explains data placement in transaction witnesses.

From RFC of NervosDAO, the deposit block header index is placed into witness_args.input_type. See implementation:
https://github.com/nervosnetwork/ckb-system-scripts/blob/a7b7c75662ed950c9bd024e15f83ce702a54996e/c/dao.c#L63C12-L63C40

In the same cell, if lock scripts like secp256k1/blake2b is used, this field(input_type) is covered by sighash_all and can't be modified. However, in iCKB, the index value is malleable. For example, suppose Alice sends a transaction with input_type set to i. An attacker could intercept and change this i to j, while still keeping the transaction valid.

The text was updated successfully, but these errors were encountered:

phroi · 2024-08-15T10:14:11Z

Hey @XuJiandong, thank you for publicly expressing your interest in iCKB by reviewing the L1 scripts source code, I personally appreciate a lot!! 🙏

I'd say that this is 100% Nervos DAO responsibility, not iCKB as Nervos DAO RFC never mentioned any requirement on the lock.

Additionally, I'd like to point out that every transition of the iCKB protocol implicitly use signature based locks, same as in Nervos DAO.

Anyway, worst case scenario, the malicious tx will not validate. Say it does validate, let me know, I would run super fast to submit a vulnerability with the Nervos Bug Bounty!! 🤣

@XuJiandong does this sound reasonable?

XuJiandong · 2024-08-15T12:27:43Z

The iCKB logic lock script doesn't rely on signature-based verification. This means that all witnesses within the same group are malleable.

phroi · 2024-08-15T14:05:31Z

The iCKB logic lock script doesn't rely on signature-based verification. This means that all witnesses within the same group are malleable.

That's 50% percent correct, let me fix it up for you:

The Owned Lock side of Owned-Owner Script doesn't rely on signature-based verification on their specific input cell(s) Withrawal Request(s). This means that all witnesses within the same group are malleable.

Because this happens at the second withdrawal step, where the main iCKB Script doesn't appear at all.

phroi · 2024-08-15T16:01:08Z

I went ahead and tried out your finding on testnet.

Methodology

I modified the signer function I currently use in v1-interface into the following:

  async function signer(tx: TransactionSkeletonType) {
    tx = prepareSigningEntries(tx, { config });

    const ethereum = getEthereum()!;

    let signedMessage = (await ethereum.request({
      method: "personal_sign",
      params: [ethereum.selectedAddress, tx.signingEntries!.get(0)!.message],
    })) as string;

    let v = Number.parseInt(signedMessage.slice(-2), 16);
    if (v >= 27) v -= 27;
    signedMessage =
      "0x" + signedMessage.slice(2, -2) + v.toString(16).padStart(2, "0");

    const signedWitness = hexify(
      OmnilockWitnessLock.pack({
        signature: bytify(signedMessage).buffer,
      }),
    );
    tx = addWitnessPlaceholder(tx, accountLock, signedWitness);

    console.log(JSON.stringify(tx, null, 2));

    // Let's maliciously change the WR witnesses
    for (const [i, c] of tx.inputs.entries()) {
      if (!isDaoWithdrawalRequest(c, config)) {
        continue;
      }

      //Input cell at index i contains a WR, let's mess up its witnesses
      const w = WitnessArgs.unpack(tx.witnesses.get(i)!);
      const j = Uint64.unpack(w.inputType!);
      tx = tx.update("witnesses", (ww) =>
        ww.set(
          i,
          hexify(
            WitnessArgs.pack({
              ...w,
              inputType: Uint64.pack(Number(j + 1n) % tx.headerDeps.size),
            }),
          ),
        ),
      );
    }

    console.log(JSON.stringify(tx, null, 2));

    return createTransactionFromSkeleton(tx);
  }

Findings

Of course the tx fails. I'd expect it to fail a Nervos DAO validation, but this fails at an earlier stage which is slightly unexpected:

Error: {"code":-1108,"message":"PoolRejectedMalformedTransaction: Malformed InvalidOutPoint transaction","data":"Malformed(\"InvalidOutPoint\", \"expect (outputs capacity) <= (inputs capacity)\")"}

Conclusion

Malleability of deposit block header index does not seem to pose a threat at the moment, then again to be 100% sure we need to carefully investigate the Nervos DAO source code.

phroi · 2024-08-15T16:03:02Z

Appendix A: valid tx (still not broadcasted for now)

{
  "cellProvider": null,
  "cellDeps": [
    {
      "outPoint": {
        "txHash": "0xe4f26ba0bd0557b643bd2050c998b407f852e82a32a44338536514454a96ee3c",
        "index": "0x0"
      },
      "depType": "depGroup"
    },
    {
      "outPoint": {
        "txHash": "0xec18bf0d857c981c3d1f4e17999b9b90c484b303378e94de1a57b0872f5d4602",
        "index": "0x0"
      },
      "depType": "code"
    }
  ],
  "headerDeps": [
    "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
    "0x8ad3e3e88d4b12c1446865395b8d236ee90c06d001eac5beb66c9235504fb8db",
    "0x65d4fbb2376e3e93df2efcd39203d76889d0bbae3a4e5a8da387fe12d7fbb9f2"
  ],
  "inputs": [
    {
      "cellOutput": {
        "capacity": "0x1726f72b85",
        "lock": {
          "codeHash": "0x9eb7a9348ff7ea510a5296b761f4302e57681f1347bcbe47203a9f83cb355f91",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x0000000000000000000000000000000001000000fe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded00000000000000000000000000000000000000000000c16ff28623002a2ba7ab445c280021",
      "outPoint": {
        "txHash": "0xa159d1e8f3c049012696535e4f6261e97bb1bb9087d5cbd24f44cedcb4680d4a",
        "index": "0x4"
      },
      "blockNumber": "0xd80ccd",
      "txIndex": "0x3"
    },
    {
      "cellOutput": {
        "capacity": "0x23c346000",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x9eb7a9348ff7ea510a5296b761f4302e57681f1347bcbe47203a9f83cb355f91",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x0"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0xcf9aa6fd2",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        }
      },
      "data": "0x",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x3"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0x372261400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x00507d50c13e00000000000000000000",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x2"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0xa4d590b542f",
        "lock": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x82d76d1b75fe2fd9a27dfbaa65a039221a380d76c926f378d3f81cf3e7e13f2e",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x4227ce0000000000",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x1"
      },
      "blockHash": "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0xa4d58d16aae",
        "lock": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x82d76d1b75fe2fd9a27dfbaa65a039221a380d76c926f378d3f81cf3e7e13f2e",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x0927ce0000000000",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x0"
      },
      "blockHash": "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0x2540be400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0xfeffffff",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x3"
      },
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0x2540be400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0xfeffffff",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x2"
      },
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    }
  ],
  "outputs": [
    {
      "cellOutput": {
        "capacity": "0x372261400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x00507d50c13e00000000000000000000"
    },
    {
      "cellOutput": {
        "capacity": "0x14da655fb1f6",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        }
      },
      "data": "0x"
    }
  ],
  "witnesses": [
    "0x10000000100000001000000010000000",
    "0x690000001000000069000000690000005500000055000000100000005500000055000000410000006a9f583a1b5a9ee91f115ee229298832212c7b127d896b6d0e10de88cd97f9b8796ff7241acda188dec91394a2a46acb12261197e8fd2f9c909d1654deef3e4500",
    "0x10000000100000001000000010000000",
    "0x10000000100000001000000010000000",
    "0x1c00000010000000100000001c000000080000000100000000000000",
    "0x1c00000010000000100000001c000000080000000200000000000000"
  ],
  "fixedEntries": [
    {
      "field": "cellDeps",
      "index": 1
    },
    {
      "field": "headerDeps",
      "index": 2
    }
  ],
  "signingEntries": [
    {
      "type": "witness_args_lock",
      "index": 1,
      "message": "0x8d93c43c69dd56b449344f880ee6a92c439bb55e22333895c378edb68075e504"
    }
  ],
  "inputSinces": {
    "4": "0x20070806e10023fe",
    "5": "0x20070806a80023fe"
  }
}

phroi · 2024-08-15T16:03:54Z

Appendix B: invalid tx

{
  "cellProvider": null,
  "cellDeps": [
    {
      "outPoint": {
        "txHash": "0xe4f26ba0bd0557b643bd2050c998b407f852e82a32a44338536514454a96ee3c",
        "index": "0x0"
      },
      "depType": "depGroup"
    },
    {
      "outPoint": {
        "txHash": "0xec18bf0d857c981c3d1f4e17999b9b90c484b303378e94de1a57b0872f5d4602",
        "index": "0x0"
      },
      "depType": "code"
    }
  ],
  "headerDeps": [
    "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
    "0x8ad3e3e88d4b12c1446865395b8d236ee90c06d001eac5beb66c9235504fb8db",
    "0x65d4fbb2376e3e93df2efcd39203d76889d0bbae3a4e5a8da387fe12d7fbb9f2"
  ],
  "inputs": [
    {
      "cellOutput": {
        "capacity": "0x1726f72b85",
        "lock": {
          "codeHash": "0x9eb7a9348ff7ea510a5296b761f4302e57681f1347bcbe47203a9f83cb355f91",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x0000000000000000000000000000000001000000fe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded00000000000000000000000000000000000000000000c16ff28623002a2ba7ab445c280021",
      "outPoint": {
        "txHash": "0xa159d1e8f3c049012696535e4f6261e97bb1bb9087d5cbd24f44cedcb4680d4a",
        "index": "0x4"
      },
      "blockNumber": "0xd80ccd",
      "txIndex": "0x3"
    },
    {
      "cellOutput": {
        "capacity": "0x23c346000",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x9eb7a9348ff7ea510a5296b761f4302e57681f1347bcbe47203a9f83cb355f91",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x0"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0xcf9aa6fd2",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        }
      },
      "data": "0x",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x3"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0x372261400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x00507d50c13e00000000000000000000",
      "outPoint": {
        "txHash": "0xfe2871a68ce9507da06725369377917643e299cfd346a65a202e3f7be0092ded",
        "index": "0x2"
      },
      "blockNumber": "0xd805ee",
      "txIndex": "0x1"
    },
    {
      "cellOutput": {
        "capacity": "0xa4d590b542f",
        "lock": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x82d76d1b75fe2fd9a27dfbaa65a039221a380d76c926f378d3f81cf3e7e13f2e",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x4227ce0000000000",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x1"
      },
      "blockHash": "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0xa4d58d16aae",
        "lock": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        },
        "type": {
          "codeHash": "0x82d76d1b75fe2fd9a27dfbaa65a039221a380d76c926f378d3f81cf3e7e13f2e",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0x0927ce0000000000",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x0"
      },
      "blockHash": "0xc81d894b58363e217139a789e9ab301c103a49402ab66d55f7294cdedd9446d5",
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0x2540be400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0xfeffffff",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x3"
      },
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    },
    {
      "cellOutput": {
        "capacity": "0x2540be400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0xe62f827ba2d36c901fad07762f28b298c1193b1989e4d47b7901e709fc4f8c13",
          "hashType": "type",
          "args": "0x"
        }
      },
      "data": "0xfeffffff",
      "outPoint": {
        "txHash": "0xe6cf5d637344cac33d51520f6961ed9988366097f7ee37c58e49d636905e1bbf",
        "index": "0x2"
      },
      "blockNumber": "0xd80108",
      "txIndex": "0x2"
    }
  ],
  "outputs": [
    {
      "cellOutput": {
        "capacity": "0x372261400",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        },
        "type": {
          "codeHash": "0x50bd8d6680b8b9cf98b73f3c08faf8b2a21914311954118ad6609be6e78a1b95",
          "hashType": "data1",
          "args": "0xb93e98a1e3ed7eda6f15d9e11bd780ab9c5218dfb413e355460458227ce5d5cd00000080"
        }
      },
      "data": "0x00507d50c13e00000000000000000000"
    },
    {
      "cellOutput": {
        "capacity": "0x14da655fb1f6",
        "lock": {
          "codeHash": "0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb",
          "hashType": "type",
          "args": "0x012fccf1486c89590ce4a08b0fca9a6a7088cdff2600"
        }
      },
      "data": "0x"
    }
  ],
  "witnesses": [
    "0x10000000100000001000000010000000",
    "0x690000001000000069000000690000005500000055000000100000005500000055000000410000006a9f583a1b5a9ee91f115ee229298832212c7b127d896b6d0e10de88cd97f9b8796ff7241acda188dec91394a2a46acb12261197e8fd2f9c909d1654deef3e4500",
    "0x10000000100000001000000010000000",
    "0x10000000100000001000000010000000",
    "0x1c00000010000000100000001c000000080000000200000000000000",
    "0x1c00000010000000100000001c000000080000000000000000000000"
  ],
  "fixedEntries": [
    {
      "field": "cellDeps",
      "index": 1
    },
    {
      "field": "headerDeps",
      "index": 2
    }
  ],
  "signingEntries": [
    {
      "type": "witness_args_lock",
      "index": 1,
      "message": "0x8d93c43c69dd56b449344f880ee6a92c439bb55e22333895c378edb68075e504"
    }
  ],
  "inputSinces": {
    "4": "0x20070806e10023fe",
    "5": "0x20070806a80023fe"
  }
}

phroi · 2024-08-15T16:45:28Z

P.S.: tangentially, from your first comment:

From RFC of NervosDAO, the deposit block header index is placed into witness_args.input_type. See implementation:
https://github.com/nervosnetwork/ckb-system-scripts/blob/a7b7c75662ed950c9bd024e15f83ce702a54996e/c/dao.c#L63C12-L63C40

This is 100% NOT the on-chain Nervos DAO, check out the commit timestamp.

Background story: last year I discovered how the limitation of only 64 output cells in Nervos DAO txs was hampering iCKB, so I forked Nervos DAO, removed that artificial limitation and created a pull request. @xxuejie then fixed up a few things and what you linked is the result.

But the script deployed on-chain is still the original one. You can double check with the commit timestamp.

And the one we should use for this review is the revision just one commit later. It has the very same code except it greatly improve comments.

Please, @XuJiandong be careful about which Nervos DAO revision you use 🙏🙏🙏

phroi · 2024-08-15T17:24:58Z

P.P.S.: from your first comment:

For example, suppose Alice sends a transaction with input_type set to i. An attacker could intercept and change this i to j, while still keeping the transaction valid.

This is an incorrect statement, after tampering the transaction, the new transaction is invalid, let me show you how.

Let's assume that by miracle Alice tx passes the tx pool check, it will be invalid as per Nervos DAO validation:

Alice tx fails step number 3. Alice took a valid tx and modified it into an invalid one. In the same line of reasoning, Alice could take a valid tx, modify the signature and the modified tx would still fail validation, just one of a different script.

phroi · 2024-08-15T17:25:52Z

@XuJiandong does all this sound reasonable?

XuJiandong · 2024-08-16T03:20:17Z

There are three fields in the witness that can be modified within the same script group:

lock
input_type
output_type

The first field is used by the iCKB logic lock script, which can be left empty. This work is optional. I don't think it causes any issues at the moment.

The input_type and output_type fields are used by Nervos DAO and xUDT. Based on your analysis, Nervos DAO is safe. For xUDT, it is also safe since no witness is involved. However, in the future, if any new type script is introduced, it should be carefully reviewed. Specifically, if the witness is modified, is the script still secure?

phroi · 2024-08-16T08:08:00Z

The input_type and output_type fields are used by Nervos DAO and xUDT. Based on your analysis, Nervos DAO is safe. For xUDT, it is also safe since no witness is involved

Yes, I wanted exactly to stress out this: at the moment this is safe. Kind of staking my reputation on this 😎

However, in the future, if any new type script is introduced, it should be carefully reviewed. Specifically, if the witness is modified, is the script still secure?

Ok, let's take a step back. This issue seems to be a weakness of my novel pattern of using a script as both lock in one cell and type into another cell. This means all the scripts are affected: iCKB Script, Limit Order Script and Owned-Owner Script. Let's examine the possible script interactions!

phroi · 2024-08-16T08:16:14Z

iCKB Script as Lock

If iCKB script validates, it will flag any output cell where iCKB Script is used as lock and anything that is not a Nervos DAO deposit as type. This will not change in the future.

That said a tx could just put as output an iCKB Script lock and anything as type, as long as iCKB Script doesn't appear as type or input lock. This can't really be prevented as output locks do not validate. This is more of a hack and it will just lock that cell forever or mint a deposit without a receipt, kind of a useful way of burning CKB.

This is 100% safe as witnesses are never used in this scenario. Say witness tampering occurs, not really an issue as far as I can see.

phroi · 2024-08-16T10:13:38Z

Owned-Owner Script as Lock

Owned-Owner Script validates only against simple Script misuses, but it doesn't check that the cell type script is exclusively a WR. Given your valid point about preventing future issues with witness tampering, I can add the check that only WR are used.

That said, a tx could just put as output an Owned-Owner Script lock and anything as type, as long as Owned-Owner Script doesn't appear as type or input lock. This can't really be prevented as output locks do not validate. This is more of a hack and it will just lock that cell forever.

In the current use case this is safe as documented earlier. Additional future issues can be prevented by locking down the type to only WR. Say witness tampering occurs on WR, not really an issue as examined before.

phroi · 2024-08-16T11:03:27Z

Limit Order Script as Lock

Limit Order Script validates only against simple Script misuses and the cell type cannot be locked down as the script aims to be compatible with sUDT, xUDT (partially) and all UDTs isomorphic to sUDT cell data and that do not use the witness.

An hypothetical solution could be to check the witness, but it would be unnecessarily complex as CoBuild shows that the witness format could change many times in the future.

That said, generally speaking the Limit Order Script already works in the possibly most hostile environment as anyone, including bad actors, can match a LO. If these hypothetical bad actors could find a way to exploit a LO, they would. Storing data on Witness is the possibly unsafest way to store data in Nervos L1. If a UDT needs to store data on Witness, then for good measure it should not be used in conjunction with LO. Given your valid point about preventing future issues with witness tampering, I'll add this very information to the proposal.

As usual, a tx could just put as output an Limit Order Script without its Master cell. This can't really be prevented as output locks do not validate. This is more of a hack and it will just lock that cell forever.

In the current use case this is safe as agreed earlier. Additional future issues may arise if a LO cell need to store data in the witness, but the proposal will explicitly warn against such failure modes. If a dev will choose anyway that path he'll do so at his own risk, similarly to creating a LO without its Master cell.

…b/v1-core#10

phroi · 2024-08-18T19:43:30Z

I restricted the Owned Owner usage to Withdawal Requests, feel free to review the changes 🤗

And I added the following section at the end of the proposal:

Warning: Unsigned Lock Witnesses Malleability ⚠️

All the script presented in this proposal (iCKB Script, Owned Owner Script and Limit Order Script) follow a novel pattern of using a script both as lock in one cell and type into another cell. While the pattern allows great flexibility, it also comes with an implicit weakness: the cell that uses the script as lock doesn't rely on signature-based verification, so the witnesses in the same group (lock, input type and output type) can be modified by an attacker after user signature. Credits to @XuJiandong for the discovery.

Rule of thumb: if a script in a transaction needs to store data in the witness and this data can be tampered without the transaction becoming invalid, then this transaction must not employ the scripts presented in the current proposal.

This witnesses malleability doesn't affect the current iCKB use-cases as no data that can be freely tampered is ever stored into witnesses.

phroi · 2024-08-18T19:45:33Z

@XuJiandong are these measures enough? Are you able to propose any alternative solutions?

phroi · 2024-08-19T02:33:56Z

Thank you again @XuJiandong for taking your time to understand and review the iCKB Scripts 🙏

I'd also like to congratulate you on the discovery of this subtle weakness!! 🎉

While it does not directly impact the current iCKB use-cases, it's a good catch for other projects who later on would like to reuse these locks.

phroi · 2024-08-25T10:15:56Z

I decided on re-opening this issue: while it doesn't impact the iCKB DApp, this Github issue serves as evident warning when using these scripts (iCKB Script, Owned Owner Script and Limit Order Script) outside the iCKB DApp, especially in conjuction with sighash_all-based locks.

phroi · 2024-09-05T11:17:14Z

@msjyryxdzzj @jlguochn when you fully understand the iCKB proposal and Scripts, feel free to evaluate this weakness of the Scripts

phroi changed the title ~~Malleability of deposit block header index~~ Malleability of unsigned lock witnesses Aug 16, 2024

phroi added a commit that referenced this issue Aug 18, 2024

Group common utils and add WR check for #10

0f09aa7

phroi added a commit to ickb/proposal that referenced this issue Aug 18, 2024

Resctrict use of Owned-Owner to WR as per ickb/v1-core#10

d874303

phroi added a commit to ickb/proposal that referenced this issue Aug 18, 2024

Add Warning on Limit Order Script as per ickb/v1-core#10

4271007

phroi added a commit to ickb/proposal that referenced this issue Aug 18, 2024

Generalize Warning on Unsigned Lock Witnesses Malleability as per ick…

afc5ef4

…b/v1-core#10

XuJiandong closed this as completed Aug 19, 2024

phroi reopened this Aug 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Malleability of unsigned lock witnesses #10

Malleability of unsigned lock witnesses #10

XuJiandong commented Aug 15, 2024

phroi commented Aug 15, 2024 •

edited

Loading

XuJiandong commented Aug 15, 2024

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024

XuJiandong commented Aug 16, 2024

phroi commented Aug 16, 2024 •

edited

Loading

phroi commented Aug 16, 2024

phroi commented Aug 16, 2024

phroi commented Aug 16, 2024

phroi commented Aug 18, 2024

Warning: Unsigned Lock Witnesses Malleability ⚠️

phroi commented Aug 18, 2024

phroi commented Aug 19, 2024

phroi commented Aug 25, 2024

phroi commented Sep 5, 2024

Malleability of unsigned lock witnesses #10

Malleability of unsigned lock witnesses #10

Comments

XuJiandong commented Aug 15, 2024

phroi commented Aug 15, 2024 • edited Loading

XuJiandong commented Aug 15, 2024

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024 • edited Loading

Methodology

Findings

Conclusion

phroi commented Aug 15, 2024 • edited Loading

Appendix A: valid tx (still not broadcasted for now)

phroi commented Aug 15, 2024 • edited Loading

Appendix B: invalid tx

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024

phroi commented Aug 15, 2024

XuJiandong commented Aug 16, 2024

phroi commented Aug 16, 2024 • edited Loading

phroi commented Aug 16, 2024

iCKB Script as Lock

phroi commented Aug 16, 2024

Owned-Owner Script as Lock

phroi commented Aug 16, 2024

Limit Order Script as Lock

phroi commented Aug 18, 2024

Warning: Unsigned Lock Witnesses Malleability ⚠️

phroi commented Aug 18, 2024

phroi commented Aug 19, 2024

phroi commented Aug 25, 2024

phroi commented Sep 5, 2024

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 15, 2024 •

edited

Loading

phroi commented Aug 16, 2024 •

edited

Loading