phroi
commented
4 weeks ago Hey @mohanson, thank you for publicly expressing your interest in iCKB by reviewing the L1 scripts source code, I personally appreciate a lot!! 🙏
This is an interesting hypothetical failure mode, pretty neat, keep this kind of issues coming!!
This attack in particular was already considered when I implemented the contracts, so let's examine together how and why it would not succeed.
I can construct a withdrawal request to extract 1152 ckb (because 1152 * 10000000000000000 / 11509953685250771 == 1000) and withdraw successfully
If ever you tried out the testnet DApp, you may get the impression that a withdrawal request of any size can occur, then again this is an abstraction that only works thanks to the limit order script & fulfillment bot, while the underlying core iCKB protocol works very differently. Also limit order script do not use division, so this attack doesn't apply there.
As described in the Withdrawal section of the iCKB proposal, an iCKB withdrawal request can choose from the iCKB Deposit pool from 1 up to 64 (64 only in a no change cells tx) of iCKB Deposits to withdraw from.
How can you be sure that deposit of exactly 1000 iCKB exists?
It would be unlikely that such deposit exist as a user creating it would lose 82 CKB in unaccounted unoccupied capacity by design, around 7% of the deposited amount.
Side Note about lose 82 CKB: If the user user creates the deposit and right after he withdraws from it, nothing is lost. Then again if someone else use that deposit to withdraw, then those 82 CKB are lost forever to the other user. This is by design to incentivize standard deposits.
Moving on, let's assume that by miracle a multitude of 1000 iCKB deposits exists, the attack still does not apply.
If you notice in the iCKB Script, the information always flow is one direction, from the CKB value to the iCKB value, never the other way around:
fn deposit_to_ickb(index: usize, source: Source, amount: u64) -> Result<u128, Error> {
let amount = u128::from(amount);
let ar_0 = GENESIS_ACCUMULATED_RATE;
let ar_m = u128::from(extract_accumulated_rate(index, source)?);
let ickb_amount = amount * ar_0 / ar_m;
// Apply a 10% discount for the amount exceeding the soft iCKB cap per deposit
if ickb_amount > ICKB_SOFT_CAP_PER_DEPOSIT {
return Ok(ickb_amount - (ickb_amount - ICKB_SOFT_CAP_PER_DEPOSIT) / 10);
}
return Ok(ickb_amount);
}The amount parameter represents the deposit unoccupied capacity, also tracked exactly by the receipt. Also given a particular deposit or a receipt cell, its accumulated rate never changes as it depends on the cell inclusion block. This means that iCKB amounts representing deposits and receipts are always deterministically calculated with the very same operations. Again, no function that converts iCKB amounts to CKB amounts exists in the code.
@mohanson does all this sound reasonable? Do you have any additional doubts?
mohanson
The attack exploits the fact that integer division always rounds down. Attack happens here: https://github.com/ickb/v1-core/blob/master/scripts/contracts/ickb_logic/src/entry.rs#L76
The attack mainly occurs in the withdraw phase.
At this time, on the Nervos mainnet, the value of dao_ar is 11509953685250771.
This is the simplest way to use this code to attack ickb. By constructing transactions in a clever way, I think it is also possible to mint ickb infinitely (1000 ickb => 1152 ckb => 1001 ickb, e.g).