Underhanded Solidity Contest 2022の優勝作品がおもしろい
式の評価順序が不定であることを悪用したバックドア付きAMMが優勝
Underhanded Solidity Contest 2022について
https://gyazo.com/7a8289e9840f22e79a70dd958641f253
優勝作品
レギュレーションを満たした提出は全部で18件あったらしい。
ChainSecurityのセキュリティエンジニアTynan Richards氏の作品。
審査員らが誰も手作業によるコードレビューでバックドアを発見できなかったらしい。それで絶賛している。「absolute winner」とまで言っている。
提出されたDEXはUniswapライクなAMMで、管理者と流動性提供者がいる。流動性提供者だけでなく、管理者も手数料の一部を貰える。トレードの手数料によって流動性がどれくらい増加したかを記録しておき、管理者はバッチで管理手数料を計算して引き出せる。その際に用いる「手数料のうちの管理手数料の割合」は再設定可能。ただ1億%とかに再設定された直後に管理手数料を引き出されると、全部の資産が引き出されるかもしれないから、再設定してから7日間は管理者は手数料を請求できない。それまで流動性提供者に流動性を引き出せる猶予期間が与えられる。
で、バックドアはこれ。
code:sol(js)
event AdminFeeChanged(uint256 indexed oldFee, uint256 indexed newFee);
function changeAdminFees(uint256 newAdminFee) external onlyAdmin nonReentrant {
emit AdminFeeChanged(retireOldAdminFee(), setNewAdminFee(newAdminFee));
}
function retireOldAdminFee() internal returns (uint256) {
_claimAdminFees();
nextFeeClaimTimestamp = block.timestamp + 7 days;
return adminFee;
}
function setNewAdminFee(uint256 newAdminFee) internal returns (uint256) {
adminFee = newAdminFee;
return newAdminFee;
}
管理者がchangeAdminFeesで手数料を変更しようとすると、AdminFeeChangedイベントが走ろうとする。次にここで引数のretireOldAdminFeeとsetNewAdminFeeの2つの関数を評価するわけだけど、どちらが先に評価されるだろうか?本来実現されるべき処理は、retireOldAdminFeeが実行され、今までの管理手数料を請求して、setNewAdminFeeで新しい手数料が設定すれることだけど、本当にそうなるだろうか?
一般的にプログラミング言語において式の評価順序は未規定動作と関わってきて嫌な気持ちになる(C言語とか)。Solidityは公式ドキュメントに書いてある通り、式の評価順序は不定。厳密には決定的に順序は決まるけど、どういったケースでどういった順序になるかが明示されていない。だけど通常f(g(),h())とあった場合、左のg()が先に評価されることが多い。だから、このコントラクトもretireOldAdminFeeが先に評価される前提で読む人が多いと思う。事実、審査員はそう読んだ。 でも実際は、右のh()が先に評価されてしまう。indexed属性が付くと右から左に逆になってしまう。要は
左から右に評価: event f(uint256 x, uint256 y);
右から左に評価: event f(uint256 indexed x, uint256 indexed y);
ということ。一応手元で実際に試してそうなったことを確認した。
これは慣れているほど注意しなくなると思う。未規定動作をよく気にするプログラミング言語(C言語とか)をやったことがあって、かつ、あんまりSolidityやってなければ、コード読んだときに「これどっちだ?」って調べたり試したりして解決すると思うけど、長くSolidityに携わっている人ほど見逃してしまう。これがバックドアじゃなくてバグならテストの段階(Fuzzerとか使って)で見つかるけど(そもそもこんな危なそうなコードはかかないけど)、そういうのは開発者(と監査機関)がやるもので、開発者側が意図的にバックドアとしてこれを仕込んだ場合、ユーザーはもう気づけない。ほとんどのユーザーはソース読まんけど。