# Putty Contest
- [Github](https://github.com/code-423n4/2022-06-putty-findings)
## Putty Contest Detail
- [Putty Contest Detail](https://github.com/code-423n4/2022-06-putty)
- [Code4rena](https://code4rena.com/reports/2022-06-putty/)
## 概要
> NFTとERC20のオーダーブック・ベースのアメリカン・オプション市場。
- ChatGPTに「An order-book based american options market」を聞いた結果。
> オーダーブック方式のアメリカン・オプション市場とは、アメリカン・オプションをオーダーブックを用いて取引する金融市場である。このような市場では、アメリカン・オプションの買い手と売り手が、特定の価格で買いまたは売りの注文を出すことができ、これらの注文は電子オーダーブックに収集される。この市場は、主催する取引所の規則に従って運営されており、トレーダーは透明で効率的な方法でアメリカン・オプションを売買することができる。アメリカン・オプションは、ヨーロピアン・オプションが満期日にしか行使できないのに対し、満期日前であればいつでも行使できるオプションである。
### 流れ
ユーザーが作成できるオフチェーン注文は以下の4種類。
- ロングプット（売ることができる権利を買う。）
- ショートプット（売ることができる権利を売る。）
- ロングコール（買うことができる権利を買う。）
- ショートコール（買うことができる権利を売る。）
注文が成立すると、２つのオプション契約がNFTの形式で作成される。
一方のNFTはショートポジションで、もう一方のNFTはロングポジションを表す。
全てのオプションは完全に担保され、物理的に決済される。
### 流れの例
プットオプションのロングを満たすための流れを紹介。
1. Aliceは期間30日権利行使価格124WETH、プレミアム0.8ETHのBoard Apesフロア2枚のロングプットオプション注文をオンチェーンで作成し署名する。
2. BobはAliceの注文を受け、`fillOrder()`を使用して**Putty**コントラクトで提出することで条件を満たす。
3. BobはWETHに変換されたストライクをカバーするために124ETHを送信する。0.8WETHがAliceのウォレットからBobのウォレットに転送される。
4. AliceにロングNFTが、BobにショートNFTが送られ、この取引でのポジションを表す。
5. 17日が経過し、Bored Apesのフロアプライスは54ETHまで下落。
6. Aliceはロングプット契約を行使し、16ETHの利益を確定することを決定。
7. AliceはBAYC #541とBAYC #8765を公開市場から、合計108ETHで購入。
8. Aliceは**Putty**上で`exercise()`を呼び出し、BAYC IDの[#541, #8765]を送る。
9. BAYC #541とBAYC #8765はAliceのウォレットから**Putty**に転送。
10. Aliceのロング・オプションは行使されたとマークされる (`exercisedPositions`)
11. 124WETHの権利行使はアリスに移される。
12. Aliceのロング・オプションは無効化され、焼却される。
13. 数時間後、BobはAliceがオプションを行使したことを知る。
14. BAYC #541とBAYC #8765が**Putty**からBobのウォレットに送られる。
15. BobのショートオプションNFTは無効になり、燃やされる。
高いレベルでは、4つの主要なエントリーポイントがあります。
- `fillOrder(Order memory order, bytes calldata signature, uint256[] memory floorAssetTokenIds)`
- `exercise(Order memory order, uint256[] calldata floorAssetTokenIds)`
- `withdraw(Order memory order)`
- `cancel(Order memory order)`
すべての注文は、`fillOrder`によってチェーン上で決済されるまで、オフチェーンに保存されます。
[`./contracts/spec`](https://github.com/code-423n4/2022-06-putty/tree/main/contracts/spec)にもっと厳密な仕様ファイルがあり、図も含まれています。
<hr>
## [H-1] プット行使時ではなく、プット失効時に手数料が差し引かれる
### ■ カテゴリー
ERC20
### ■ 条件
1. `withdraw`が呼び出される。
2. `order.isCall`が`false`かつ`isExercised`が`false`。
### ■ ハッキングの詳細
- プット行使時ではなく、プット失効時に手数料が差し引かれてしまっている。
- `setFee()`を確認すると、ポジションが行使される（行使された）時のみ、手数料額が行使額から差し引かれるとコメントしてある。
- しかし、`wisdraw()`を確認すると、プットポジションが行使されず、執行した場合にも手数料が差し引かれている。
```solidity=
if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
// send the fee to the admin/DAO if fee is greater than 0%
uint256 feeAmount = 0;
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L495-L503
- また、`exercise()`を確認するとプットポジションが権利行使され、権利が呼び出し側に渡る際、`order.strike`から手数料が差し引かれていない。
```solidity=
} else {
// -- exercising a put option
// save the floor asset token ids to the short position
uint256 shortPositionId = uint256(hashOppositeOrder(order));
positionFloorAssetTokenIds[shortPositionId] = floorAssetTokenIds;
// transfer strike from putty to exerciser
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike);
// transfer assets from exerciser to putty
_transferERC20sIn(order.erc20Assets, msg.sender);
_transferERC721sIn(order.erc721Assets, msg.sender);
_transferFloorsIn(order.floorTokens, floorAssetTokenIds, msg.sender);
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L451
### ■ 修正方法
1. 手数料が差し引かれる条件を変更。
- before
```solidity=
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
```
- after
```solidity=
// 変更
if (fee > 0 && order.isCall && isExercised) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
```
2. プット行使・権利行使後の`feeAmount`の計算・控除部分に追加。
- before
```solidity=
// transfer strike from putty to exerciser
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike);
```
- after
```solidity=
// 追加
// ========================================================
uint256 feeAmount = 0;
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
// ========================================================
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
```
<hr>
## [H-2] ACCEPTCOUNTEROFFER()で両方の注文をされることがある
### ■ カテゴリー
Reentrancy
### ■ 条件
1. 条件を更新して再度申し込みを行う。
2. 元の注文を約定する`fillOrder()`を`acceptCounterOffer()`が実行される前に実行。
3. `acceptCounterOffer()`が`originalOrder`のキャンセル実行。
### ■ ハッキングの詳細
- ユーザーが[カウンターオファー](https://www.hrpro.co.jp/glossary_detail.php?id=108)（売り手側から提示された価格などの条件に買い手側がが満足しない際、条件を修正して再度申し込みを行う）を受け入れようとする時、`acceptCounterOffer()`をキャンセルされる`originalOrder`と新しい`order`と共に呼び出す。
- 攻撃者（もしくは同時に`fillOrder()`を呼び出したユーザー）は、`acceptCounterOffer()`が`originalOrder`をキャンセルする前に、上記を約定することが可能になる。
- その結果、`originalOrder`と`order`の両方が約定され、`acceptCounterOffer()`の`msg.sender`は、必要なトークン転送が成功すると、意図した2倍レバレッジがかかる。
- `acceptCounterOffer()`は元の注文の`cancel()`を呼び出すが、注文がすでに確定している場合は元に戻らず、将来の`fillOrder()`トランザクションの成功を阻止するだけ。
```solidity=
function acceptCounterOffer(
Order memory order,
bytes calldata signature,
Order memory originalOrder
) public payable returns (uint256 positionId) {
// cancel the original order
cancel(originalOrder);
// accept the counter offer
uint256[] memory floorAssetTokenIds = new uint256[](0);
positionId = fillOrder(order, signature, floorAssetTokenIds);
}
```
```solidity=
function cancel(Order memory order) public {
require(msg.sender == order.maker, "Not your order");
bytes32 orderHash = hashOrder(order);
// mark the order as cancelled
cancelledOrders[orderHash] = true;
emit CancelledOrder(orderHash, order);
}
```
- どのユーザーでも、元の注文を約定する`fillOrder()`トランザクションで`acceptCounterOffer()`トランザクションを前倒しで実行できる。
- その結果、ユーザーは`order`と`originalOrder`の両方を満たすことができ、`acceptCounterOffer()`は`originalOrder`をキャンセルして新しい注文を約定し、両方の注文が約定される。
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L573-L584
### ■ 修正方法
- 注文がすでに約定している場合、`cancel()`を戻すようにする。
- before
```solidity=
function cancel(Order memory order) public {
require(msg.sender == order.maker, "Not your order");
bytes32 orderHash = hashOrder(order);
// mark the order as cancelled
cancelledOrders[orderHash] = true;
emit CancelledOrder(orderHash, order);
}
```
- after
```solidity=
function cancel(Order memory order) public {
require(msg.sender == order.maker, "Not your order");
bytes32 orderHash = hashOrder(order);
// 追加
require(_ownerOf[uint256(orderHash)] == address(0), "Order already filled");
// mark the order as cancelled
cancelledOrders[orderHash] = true;
emit CancelledOrder(orderHash, order);
}
```
<hr>
## [H-3] floorTokens配列が空でないショートコール注文を作成すると、オプションの行使と撤退が不可能になる
### ■ カテゴリー
ERC721
### ■ 条件
1. `fillOrder`がショートコール時に`order.floorToken`が空でない。
### ■ ハッキングの詳細
- ショートコール注文で、`floorTokens`配列がからでないものが作成された際、テイカー（オプションの買い手）は権利を行使できない。
- また、メーカー（オプションの売り手）が引き出すこともできない。
- 注文が成立した場合、メイカーはプレミアムを獲得できる。
- 空の`floorTokens`配列が偶然含まれていた場合、テイカー・メイカー両者にとって損失。
- テイカーは権利を行使できずにプレミアムを失い、メーカーはロックされたERC20とERC721を失う。
### ■ 修正方法
- `fillOrder`がショートコール時に`order.floorToken`が空であることを保証する。
<hr>
## [H-4] Zero Strike Calls・オプションは、システム的にテイカーからプレミアムを奪うために使われる可能性がある
### ■ カテゴリー
ERC20, ERC721
### ■ 条件
1. `strike`の値が0。
2. `safeTransfer()`が実行される。
### ■ ハッキングの詳細
- システムは権利行使価格をチェックせずに、無条件に転送しようとしている。
```solidity=
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
}
```
- 悪意のないERC20の中には、金額がゼロの転送を許さないものがあり、`order.baseAsset`はその対象となる可能性がある。
- Zero Strike Calls（特定の基準価額が0になるコールオプション）は有効であり、十分に一般的なデリバティブの一種。
- しかし、`order.baseAsset`を持つZero Strike Callsは行使できないため、悪意あるメーカー（オプションの売り手）は期限切れを待ってアセットを引き出し、実質的に無料でプレミアムを徴収でき、メイカーはテイカー（オプションの買い手）から盗むことができる。
```solidity=
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the premium
require(msg.value == order.premium, "Incorrect ETH amount sent");
// convert ETH to WETH and send premium to maker
// converting to WETH instead of forwarding native ETH to the maker has two benefits;
// 1) active market makers will mostly be using WETH not native ETH
// 2) attack surface for re-entrancy is reduced
IWETH(weth).deposit{value: msg.value}();
IWETH(weth).transfer(order.maker, msg.value);
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium);
}
```
- 資産を転送する。
```solidity=
// filling short call: transfer assets from maker to contract
if (!order.isLong && order.isCall) {
_transferERC20sIn(order.erc20Assets, order.maker);
_transferERC721sIn(order.erc721Assets, order.maker);
return positionId;
}
```
- 期限切れを待って資産を回収
```solidity=
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
}
```
```solidity=
// transfer assets from putty to owner if put is exercised or call is expired
if ((order.isCall && !isExercised) || (!order.isCall && isExercised)) {
_transferERC20sOut(order.erc20Assets);
_transferERC721sOut(order.erc721Assets);
// for call options the floor token ids are saved in the long position in fillOrder(),
// and for put options the floor tokens ids are saved in the short position in exercise()
uint256 floorPositionId = order.isCall ? longPositionId : uint256(orderHash);
_transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[floorPositionId]);
return;
}
```
- Zero Strike Callsのプレミアムは通常、相当な額になる。
### ■ 修正方法
- 全てのケースで転送前に`strike`が正であることを確認する。
- before
```solidity=
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the strike
require(msg.value == order.strike, "Incorrect ETH amount sent");
// convert ETH to WETH
// we convert the strike ETH to WETH so that the logic in withdraw() works
// - because withdraw() assumes an ERC20 interface on the base asset.
IWETH(weth).deposit{value: msg.value}();
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
```
- after
```solidity=
// 追加
if (order.strike > 0) {
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the strike
require(msg.value == order.strike, "Incorrect ETH amount sent");
// convert ETH to WETH
// we convert the strike ETH to WETH so that the logic in withdraw() works
// - because withdraw() assumes an ERC20 interface on the base asset.
IWETH(weth).deposit{value: msg.value}();
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike);
}
}
```
<hr>
## [M-1] 悪質なトークン契約により注文がロックされる可能性がある
### ■ カテゴリー
ERC20, ERC721
### ■ 条件
1. 攻撃者が注文を作成。
2. 以下のうちどれか1つを攻撃者のコントロール化にあるコントラクトに設定。
3. `exercise()`が実行されないようにする。
4. 攻撃者が`withdraw()`を実行。
### ■ ハッキングの詳細
- 以下のいずれかのフィールドに悪意のあるトークンコントラクトを含ませることで、`excercise()`や`written()`を実行させないようにすることが可能。
- baseAsset
- floorTokens[]
- erc20Assets[]
- erc721Assets[]
- 攻撃者は注文を作成し、そのアドレスの1つを攻撃者のコントロール下にある悪意のあるコントラクトに設定できる。
- 攻撃者は、ユーザーが注文を満たすことを許可。
- 攻撃者は、望ましくないポジションにある場合（例えば、ショートして価格が上昇した場合）、注文が`exercise()`（行使）されるのを防ぐことによって利益を得る。
- 攻撃者は、時間切れか価格が下がるのを待ち、悪意のあるトークンで転送が行われるようにする。
- `withdraw()`も信頼できない外部アドレスに呼び出されるため、同様の攻撃が可能。
- この場合、攻撃者はオプションを行使して、他のユーザーがNFTまたはERC20トークンを要求できないようにすることができる。
### ■ 修正方法
- ユーザーが悪意のあるコントラクトを設定できないように、承認されたERC20トークンまたはERC721アドレスコントラクトのホワイトリスト化。
- ただし、管理者の入力とガスのトレードオフが大きく影響してくる。
<hr>
## [M-2] 無制限ループが原因でEXERCISE()やWITHDRAW()が失敗することがある
### ■ カテゴリー
Dos
### ■ 条件
1. `transfer()`実行時にガスコストやガス上限に引っかかる。
### ■ ハッキングの詳細
- 注文で転送されるトークンの数に制限はなく、ガスの必要量も変わることがあるため、（特に注文の期間は27年間）、時間T1に約定された注文は時間T2には行使/引き出しができない可能性があり、資産が転送中に多くのガスを使用する場合（例えばaTokenやcToken）、提供された資産では行使できないかもしれない。
- オプションの買い手はプレミアムを支払っているのに、借りているアセットを手に入れることができないことになる。
- 以下のループで転送される資産の数に上限はない。
```solidity=
function _transferERC20sOut(ERC20Asset[] memory assets) internal {
for (uint256 i = 0; i < assets.length; i++) {
ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);
}
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L636-L640
```solidity=
function _transferERC721sOut(ERC721Asset[] memory assets) internal {
for (uint256 i = 0; i < assets.length; i++) {
ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);
}
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L646-L650
```solidity=
function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {
for (uint256 i = 0; i < floorTokens.length; i++) {
ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);
}
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L657-L661
### ■ 修正方法
- 資産の数に上限を設けるか、必要に応じて1つずつ転送できるようにする。
<hr>
## [M-3] プットオプションの売り手は、ゼロ金額または存在しないトークンを指定することで行使を防ぐことができる
### ■ カテゴリー
ERC20
### ■ 条件
1. `tokenAmount`が0。
2. `safeTransferFrom`を実行。
### ■ ハッキングの詳細
- プットオプションの買い手は、売り手に資産「プット」して、現在の市場価格ではなく、権利行使価格を得ることができるという特権のために、売り手にオプション・プレミアムを支払う。
- 「プット」を実行できない場合、買い手は何もせずにプレミアムを支払ったことになり、本質的に資金を盗まれたことになる。
- プットオプションの売り手が`order.erc20Assets`にいずれかの資産にゼロの金額を含めるか、通貨がないアドレスを指定すると、プットの買い手はオプションを行使できず、無駄にプレミアムを支払ったことになる。
```solidity=
// transfer assets from exerciser to putty
_transferERC20sIn(order.erc20Assets, msg.sender);
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L453-L454
- いずれかの金額がゼロに等しいか、または資産が存在しない場合元に戻ってしまう。
```solidity=
function _transferERC20sIn(ERC20Asset[] memory assets, address from) internal {
for (uint256 i = 0; i < assets.length; i++) {
address token = assets[i].token;
uint256 tokenAmount = assets[i].tokenAmount;
require(token.code.length > 0, "ERC20: Token is not contract");
require(tokenAmount > 0, "ERC20: Amount too small");
ERC20(token).safeTransferFrom(from, address(this), tokenAmount);
}
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L593-L603
### ■ 修正方法
- 全てのERC20アセットが0より大きいトークン量を持ち、コントラクトとして存在することをチェックする。
- before
```solidity=
require(token.code.length > 0, "ERC20: Token is not contract");
require(tokenAmount > 0, "ERC20: Amount too small");
ERC20(token).safeTransferFrom(from, address(this), tokenAmount);
}
}
...
*/
function _transferERC20sOut(ERC20Asset[] memory assets) internal {
for (uint256 i = 0; i < assets.length; i++) {
ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);
}
}
```
- after
```solidity=
require(tokenAmount > 0, "ERC20: Amount too small");
// requireの削除
// 追加
if (tokenAmount > 0) {
ERC20(token).safeTransferFrom(from, address(this), tokenAmount);
}
}
}
...
*/
function _transferERC20sOut(ERC20Asset[] memory assets) internal {
for (uint256 i = 0; i < assets.length; i++) {
// 追加
if (assets[i].tokenAmount > 0) {
ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);
}
}
}
```
<hr>
## [M-4] プットオプションは手数料無料
### ■ カテゴリー
ERC20
### ■ 条件
1. 行使されたプットオプションの`transfer()`を実行。
### ■ ハッキングの詳細
- オプションが行使されるたびに手数料が支払われることが予想される。
- プットオプションが行使されると、行使者は`order.baseAsset`建ての行使価格（当初はショートポジションの保有者から預かったもの）を受け取る。
- コールオプションが行使された場合、行使者は権利行使価格を**Putty**に送信し、ショートポジションの保有者は権利行使額を引き出すことができるようになる。
- しかし、現在のプロトコルの実装では、行使されたプットオプションの手数料を差し引くことができない。（プットオプションは手数料が無料になってしまっている）
- 行使されたコールに対して、プロトコル料金が正しく課金される。
```solidity=
// transfer strike to owner if put is expired or call is exercised
if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
// send the fee to the admin/DAO if fee is greater than 0%
uint256 feeAmount = 0;
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
return;
}
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L494-L506
- プットオプションは手数料が無料になっている。
```solidity=
// transfer strike from putty to exerciser
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike);
```
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L450-L451
### ■ 修正方法
- 行使されたプットオプションにも手数料を課す。
<hr>
## [M-5] FILLORDER()とEXERCISE()は、コントラクトに送られたETHERを永遠にロックする可能性がある
### ■ カテゴリー
ERC20
### ■ 条件
1. `payable`が設定されている、Etherを必要としない関数にEtherを渡す。
### ■ ハッキングの詳細
- `fillOrder()`と`exercise()`にはEtherを送る必要があるコードパスがあり、2つの関数には`payable`修飾子が設定されている。
- しかし、関数内にはEtherを必要としないコードパスが存在する。
- Etherを必要としないコードパスを使用した場合、関数に渡されたEtherは永遠に契約に固定され、送信者は何も得ることができない。
### ■ 修正方法
- 以下3箇所に`require(0 == msg.value)`を追加。
- before
```solidity=
// 371行目
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the premium
require(msg.value == order.premium, "Incorrect ETH amount sent");
...
// 403行目
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the strike
require(msg.value == order.strike, "Incorrect ETH amount sent");
...
// 505行目
_transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[uint256(orderHash)]);
} else {
// -- exercising a put option
// save the floor asset token ids to the short position
uint256 shortPositionId = uint256(hashOppositeOrder(order));
```
- after
```solidity=
// 371行目
// 変更
if (msg.value > 0) {
// check enough ETH was sent to cover the premium
require(msg.value == order.premium, "Incorrect ETH amount sent");
...
// 403行目
// 変更
if (msg.value > 0) {
// check enough ETH was sent to cover the strike
require(msg.value == order.strike, "Incorrect ETH amount sent");
...
// 505行目
_transferFloorsOut(order.floorTokens, positionFloorAssetTokenIds[uint256(orderHash)]);
} else {
// -- exercising a put option
// 追加
// exercising a put never needs native ETH
require(msg.value == 0, "Puts can't use native ETH");
// save the floor asset token ids to the short position
uint256 shortPositionId = uint256(hashOppositeOrder(order));
```
<hr>
## [M-6] コントラクト所有者がstrikeを引き出そうとするユーザーをブロックする可能性
### ■ カテゴリー
ERC20
### ■ 条件
1. ユーザーがトークンを引き出す際に、`owner()`のアドレスを0にする。
2. `baseAsset`がERC777トークンである
### ■ ハッキングの詳細
- Puttyコントラクトから預けていたstrikeをユーザーが引き出す際、strikeの一部を手数料として徴収する。
- コントラクト所有者は、ユーザーがコントラクトから行使額を引き落とす以下の2つの方法がある。
1. `owner()`のアドレスを0にする。
2. `baseAsset`がERC777トークンである。
- `PuttyV2`コントラクトがrecipientに料金を送信するたびに、常にリバートできてしまい誰もコントラクトから行使額を引き出すことができなくなる。
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L500
### ■ 修正方法
- 出勤時にowner addressに直接手数料を転送するのではなく、ownerが受け取ることができる手数料の金額を変数に保存し、ownerが`PuttyV2`コントラクトから手数料を引き出すことができる関数を作成。
```solidity=
mapping(address => uint256) public ownerFees;
function withdraw(Order memory order) public {
// transfer strike to owner if put is expired or call is exercised
if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
// send the fee to the admin/DAO if fee is greater than 0%
uint256 feeAmount = 0;
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ownerFees[order.baseAsset] += feeAmount
}
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
return;
}
}
function withdrawFee(address baseAsset) public onlyOwner {
uint256 _feeAmount = ownerFees[baseAsset];
ownerFees[baseAsset] = 0;
ERC20(baseAsset).safeTransfer(owner(), _feeAmount);
}
```
<hr>
## [M-7] 攻撃者は、ERC721をサポートしないNFT（CRYPTOPUNKなど）に対してショートプットのオプション注文を作成でき、ユーザーは注文を約定できるが、オプションを行使できない
### ■ カテゴリー
ERC721
### ■ 条件
1. ERC721をサポートしていないNFTを使用してショートオプションを作成。
2. `safeTransferFrom`を呼び出し。
### ■ ハッキングの詳細
- 攻撃者はCRYPTOPUNKなどのERC721をサポートしていないNFTの場合、ショートオプションを作成し、ユーザーが約定すると`baseAsset`がコントラクトに転送される。
- しかし、`safeTransferFrom`の呼び出しに失敗し、ユーザーはオプションを行使できない。
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L343-L346
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L628-L629
### ■ 修正方法
- 順番にNFTのホワイトリストに追加したり、CRYPTOPUNKなどで権利を行使できるようにサポートする。
<hr>
## [M-8] ERC721.TRANSFERFROM()とERC20.TRANSFERFROM()の重複により、ORDER.ERC20ASSETSまたはORDER.BASEASSETがERC20ではなく、ERC721である可能性
### ■ カテゴリー
ERC20, ERC721
### ■ 条件
1. `ERC20.transferFrom(address to, address from, uint256 amount)`を実行する。
### ■ ハッキングの詳細
- 以下の2つは同じ関数署名`0x23b872dd`を持っているため、`baseAsset`や`erc20Assets`がERC20ではなくERC721のアドレスになる可能性がある。
- NFTをコントラクトから転送できず、永久にコントラクト内に止まってしまうことになる。
### ■ 修正方法
1. 承認されたERC721とERC20のトークンコントラクトをホワイトリストに登録する。
2. 2つのコントラクトをERC20とERC721の異なるホワイトリストに分け、それぞれのコントラクトが正しいカテゴリにあることを確認。
<hr>
## [M-9] コントラクトは、手数料なしのフラッシュローンプールとして機能
### ■ カテゴリー
Reentrancy
### ■ 条件
1. ERC20以外のカスタムロジックを持つコントラクトを`order.baseAssets`が参照している注文で`PuttyV2.fillOrder`を呼ぶ。
### ■ ハッキングの詳細
- 悪意のあるユーザは、PuttyV2契約を利用して、資産に手数料を支払うことなくフラッシュローンを行い、利益を得ることができる。
### ■ 修正方法
<hr>
## [M-10] PUTTYのポジショントークンがERC721以外のレシーバーに鋳造される可能性
### ■ カテゴリー
ERC721
### ■ 条件
1. NFTのミント先のアドレスでERC721をサポートしていないコントラクトアドレスの時。
### ■ ハッキングの詳細
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L302-L308
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2Nft.sol#L11-L18
- `Putty`はコード全体でERC721の`safeTransfer`と`safeTransferFrom`を使用して、ERC721トークンがERC721をサポートしていないコントラクトアドレスに転送されないようにチェックしている。
- fillOrder`の最初に位置で`_mint`を使用して受けてがERC721トークン転送を受け入れるかチェックしていない。
- `PuttyV2#fillOrder`
```solidity=
// create long/short position for maker
_mint(order.maker, uint256(orderHash));
// create opposite long/short position for taker
bytes32 oppositeOrderHash = hashOppositeOrder(order);
positionId = uint256(oppositeOrderHash);
_mint(msg.sender, positionId);
```
- `PuttyV2Nft#_mint`
```solidity=
function _mint(address to, uint256 id) internal override {
require(to != address(0), "INVALID_RECIPIENT");
require(_ownerOf[id] == address(0), "ALREADY_MINTED");
_ownerOf[id] = to;
emit Transfer(address(0), to, id);
}
```
- メーカーやテイカーがERC721を受け取ることができないコントラクトであるとき、オプションポジションがロックされ譲渡できなくなってしまう。
### ■ 修正方法
- `_safeMint`に`require`のチェックを追加する。
- before
```solidity=
function _safeMint(address to, uint256 id) internal virtual {
_mint(to, id);
}
```
- after
```solidity=
function _safeMint(address to, uint256 id) internal virtual {
_mint(to, id);
// 追加
require(
to.code.length == 0 ||
ERC721TokenReceiver(to).onERC721Received(msg.sender, address(0), id, "") ==
ERC721TokenReceiver.onERC721Received.selector,
"UNSAFE_RECIPIENT"
);
}
```
<hr>
## [M-11] ユーザーの同意なしに料金を変更することができる
### ■ カテゴリー
タイミング
### ■ 条件
1. 注文約定後に手数料を変更する。
### ■ ハッキングの詳細
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L240
- https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L497
- 手数料は`withdraw`実行時に適用され、注文が成立して条件が合意されてから出勤までの間に変更される可能性があり、期待した資金を失うことにつながる。
-
### ■ 修正方法
1. 料金を`Order`に格納し、注文が約定された時に正しいかどうかを検証するために構造体を作成。
2. 手数料が0以上かチェック。
3. 過去の料金や料金変更のタイムスタンプをメモリー（配列など）に保存し、出勤時に作成時の料金を取り出せるようにする。
- before
```solidity=
ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium);
```
- after
```solidity=
// 161行目に追加
/**
@notice The total unclaimed premium fees for each asset.
*/
mapping(address => uint256) public unclaimedFees;
```
```solidity=
// 257行目に追加
/**
@notice Withdraws the fees that have been collected from premiums for a particular asset.
@param asset The asset to collect fees for.
@param recipient The recipient address for the unclaimed fees.
*/
function withdrawFees(address asset, address recipient) public payable onlyOwner {
uint256 fees = unclaimedFees[asset];
// reset the fees
unclaimedFees[asset] = 0;
emit WithdrewFees(asset, fees, recipient);
// send the fees to the recipient
ERC20(asset).transfer(recipient, fees);
}
```
```solidity=
// 追加
// collect fees
if (feeAmount > 0) {
ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), feeAmount);
}
```
<hr>
## [M-12] 権利行使価格が小さいオプションは0に切り捨てられ、資産を引き出せないことがある。
### ■ カテゴリー
ERC20
### ■ 条件
1. 小さなオプション行使と手数料を使用する。
### ■ ハッキングの詳細
- 特定のERC20トークンは値が0のトークンの転送とrevertをサポートしていない。
- かなり小さなオプション行使と低いプロトコル手数料率の`order.baseAssets`として使用すると0に切り捨てられ、ポジションの資産の引き出しができなくなる可能性がある。
- `revert`されると資産と`strike`を戻すことができずコントラクトに凍結されてしまう。
**例**
- `order.baseAsset`は、ERC20トークンの1つ。
- `order.strike = 999`(小数点以下の桁数によって、非常に小さなオプションポジション)
- `手数料＝1`（0.1）
((999 * 1) / 1000 = 0.999) = 0に切り捨て → ゼロ値移転復帰取引
### ■ 修正方法
- `feeAmount`が0以上かチェック。
- before
```solidity=
ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium);
```
- after
```solidity=
// 追加
// collect fees
if (feeAmount > 0) {
ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), feeAmount);
}
```
<hr>
## [M-13] 不正なメーカーが注文期間を0に設定できる
### ■ カテゴリー
### ■ 条件
- 注文約定時に最低注文期間を0に設定。
### ■ ハッキングの詳細
- 悪意のあるメーカーは、最低注文期間を0に設定でき、注文が成立したのち即座に執行できるてしまう。
- テイカーは引き出しオプションのみで、権利行使価格の手数料がかかるため損をしてしまう。
### ■ 修正方法
- 最低注文期間を設ける。
- 今回は15分を設けた。
- 追加（*`src/PuttyV2.sol`*）
```solidity=
// check duration is not too long
require(order.duration <= 10_000 days, "Duration too long");
// check duration is not too short
require(order.duration >= 15 minutes, "Duration too short");
```
<hr>
## [M-14] オーダーキャンセルがフロントランニングになりやすく、集中管理されたデータベースに依存
### ■ カテゴリー
Front Running
### ■ 条件
1. 注文にサインし、`Putty`のサーバーに記録される。
2. `Putty`のサーバーがオフラインになる。
3. 攻撃者が注文をコピーしている。
### ■ ハッキングの詳細
- 注文のキャンセルはメーカーがパラメータとして注文を入力し、`cancel()`を呼びあす必要がある。（唯一のキャンセル方法）
- ただし、以下の2つの問題がある。
1. MEVユーザーがキャンセルを前倒しで行い、注文を満たす。
2. 注文をキャンセルするための集中管理されたサービスへの依存。
- 注文は連鎖的に署名されるため、集中管理されたデータベースに保存される。
- エンドユーザーがした注文を全てローカルに記録することはまずあり得ない。
- つまり、注文をキャンセルする場合、メーカーは集中型サービスに注文パラメータを要求する必要がある。
- ここでもし、集中管理サービスがオフラインになった場合、注文データベースのコピーを持つアクのあるものが、他の方法でキャンセルされたはずの注文を約定できる可能性がある。
### ■ 修正方法
- 呼び出し元の全てのオーダーをキャンセルする追加メソッドを持たせる。
- ユーザーアドレスからnonceのマッピングとして`minimumValidNonce`をインクリメントできるようにする。
```solidity=
mapping(address => uint256) minimumValidNonce;
```
- ユーザーが
<hr>
## [M-15] ゼロストライクコールオプションは、システム手数料の支払いを回避できる
### ■ カテゴリー
ERC20
### ■ 条件
1. 0または0に近い権利行使価格でのコール。
### ■ ハッキングの詳細
- 0または0に近い権利行使価格のコールは、一般的なデリバティブの一種。
- このデリバティブの場合、システムは手数料を受け取らないため、手数料は注文の権利行使価格の何分の1かで計算される。
- また、OTMのコールオプションの場合、オプションそのものはほとんど価値がないのに、権利行使価格が大きいため手数料が大きくなり問題である。
- 例えば1kETHのBAYCコールに大した価値がないのに、手数料が通常の10倍というかなりの額になる。
- 手数料は注文の権利行使に連動しており、深いITM（オプション取引の買方が権利行使した時に、利益が生じる状態）とOTM（権利所有者が権利行使をしたときに損失が出る状態）のコールなど、注文の種類によって手数料が大きく異なる。
```solidity=
// transfer strike to owner if put is expired or call is exercised
if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
// send the fee to the admin/DAO if fee is greater than 0%
uint256 feeAmount = 0;
if (fee > 0) {
feeAmount = (order.strike * fee) / 1000;
ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
}
ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
return;
}
```
### ■ 修正方法
1. 手数料をオプションプレミアムと連動させる。
- before
```solidity=
// transfer premium to whoever is short from whomever is long
if (order.isLong) {
ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium);
} else {
// handle the case where the user uses native ETH instead of WETH to pay the premium
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the premium
require(msg.value == order.premium, "Incorrect ETH amount sent");
// convert ETH to WETH and send premium to maker
// converting to WETH instead of forwarding native ETH to the maker has two benefits;
// 1) active market makers will mostly be using WETH not native ETH
// 2) attack surface for re-entrancy is reduced
IWETH(weth).deposit{value: msg.value}();
IWETH(weth).transfer(order.maker, msg.value);
} else {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium);
}
}
```
- after
```solidity=
// transfer premium to whoever is short from whomever is long
if (order.isLong) {
// 変更
// transfer premium to taker
ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium - feeAmount);
// 追加
// collect fees
if (feeAmount > 0) {
ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), feeAmount);
}
} else {
// handle the case where the user uses native ETH instead of WETH to pay the premium
if (weth == order.baseAsset && msg.value > 0) {
// check enough ETH was sent to cover the premium
require(msg.value == order.premium, "Incorrect ETH amount sent");
// convert ETH to WETH and send premium to maker
// converting to WETH instead of forwarding native ETH to the maker has two benefits;
// 1) active market makers will mostly be using WETH not native ETH
// 2) attack surface for re-entrancy is reduced
// 変更
IWETH(weth).deposit{value: order.premium}();
// collect fees and transfer to premium to maker
IWETH(weth).transfer(order.maker, order.premium - feeAmount);
} else {
// 変更
// transfer premium to maker
ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium - feeAmount);
// 追加
// collect fees
if (feeAmount > 0) {
ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), feeAmount);
}
}
}
```
<hr>
## [M-16] PUTTYV2 に適用される2つの既知の問題
### ■ カテゴリー
Solidity Version
### ■ 条件
1. Solidityのバージョン`0.8.13`を使用。
2. calldataから直接ネストした配列をABIエンコードする。
3. インライン アセンブリでのメモリ書き込み操作。
### ■ ハッキングの詳細
- `solidity version 0.8.13`では、**PuttyV2**に以下の2つの問題がある。
1. ABI-encodingに関連する脆弱性
- `hashOrder()`および`hashOppositeOrder()`。
> ネストされた配列を直接別の外部関数呼び出しに渡すか、その上でabi.encodeを使用する。
- https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/
2. 「インラインアセンブリのメモリ副作用に関するオプティマイザのバグ」に関連する脆弱性
- **PuttyV2**は**openzeppelin**と**solmate**からsolidityのコントラクトを継承しており、どちらもインラインアセンブリを使用し、コンパイル時に最適化が行われている。
- https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/
### ■ 修正方法
- `solidity version 0.8.15`を使用する。
Sign in with Wallet
Connect another wallet