---
# System prepended metadata

title: Day 7

---

# Day 7



以下yoki記載

# [yoki] addApprovedTokens, addWhitelistで同じトークンを複数登録することが可能

## 要約

addApprovedTokens, addWhitelistで重複してトークン登録することが可能なので、`_isXXX`関数におけるオーバーフローが生じる。

## バグの詳細

addApprovedTokens, addWhitelistでは重複のチェックなく配列に格納しています。

## インパクト
`_isXXX`関数内において、`unchecked`を用いて加算しているので`addApprovedTokens`や`addWhitelist`の要素数が増えるとオーバーフローとなる可能性がある。

## 対象となるコード
```.sol
 function addWhitelist(address _token) public {
    if (!_isXXX(_token, approvedTokens)) revert();
    whitelist.push(_token);
  }
```

```.sol
  function addApprovedTokens(address _token) public {
    if (msg.sender != owner) revert();
    approvedTokens.push(_token);
  }
```

## 使用したツール
特になし

## 改善方法
配列にpushする前に追加対象の値外既に存在するか確認する。

# [yoki] isETHをtrueにしてETH以外のトークンを指定すること（またはその逆）が可能である

## 要約

## バグの詳細
`deposit`や`withdraw`関数にて`_isETH`を引数として持つが、
* ETHを扱うが`_isETH`を`false`
* ETHを扱わないが`_isETH`を`true`
といったリクエストが可能である。

## インパクト
例えば、`deposit`関数利用時に任意のトークンを指定しと`_isETH`を`true`とした場合にはETHの送金処理が実行されることになる。

## 対象となるコード
```.sol
  function deposit(uint _amount, address _token, bool _isETH) public payable {
```

```.sol
  function withdraw(
    address _to,
    uint _amount,
    bool _isETH,
    address _token
  ) public payable {
```

```.sol
  function _tokenTransfer(TransferInfo memory _info) private {
    if (_info.isETH) {
    emit DebugLogEvent("debug: This is eth");
      (bool success, ) = _info.to.call{ value: _info.amount }("");
      require(success, "Failed");
    } else {
     emit DebugLogEvent("debug: This is not eth");
      IERC20(_info.token).transferFrom(_info.from, _info.to, _info.amount);
    }
  }
```

## 使用したツール
特になし

## 改善方法
ETHかどうかの判別をインターフェイスに持たせるのではなく、`msg.value`の値を利用してその判別を行う。

---
以下mabukさん記載


# ETHのdepositが出来ない

## 要約
ETHをコントラクトにdeposit関数で送金すると必ずエラーとなる。つまり誰もETHをdeposit出来ない。

## バグの詳細
コントラクトのコンパイルは出来るが、この実装だとコントラクトはETHの送金を受け取れない。
ETHの送金を受け取るにはdepositはpayableであり、かつreceive関数またはfallback関数が必要。

## インパクト
ETHのdepositが必ず失敗するので誰もETHのdepositが出来ない。

## 対象となるコード
```solidity
  function deposit(uint _amount, address _token, bool _isETH) public {
    if (!_isXXX(_token, whitelist)) revert();
    DepostInfo memory depositInfo;
    TransferInfo memory info = TransferInfo({
        isETH: _isETH,
        token: _token;
        from: msg.sender, 
        amount: _amount,
        to: address(this),
    });

    _tokenTransfer(info);
    depositInfo.lastTime = uint40(block.timestamp);
    depositInfo.amount = _amount;
    depositAmt[msg.sender][_token] = depositInfo;
  }
```

## 使用したツール
hardhat

## 改善方法
- receive関数の追加
- depositにpayable修飾子を追加
```solidity
//before
function deposit(uint _amount, address _token, bool _isETH) public {

//after
receive() external payable {}
function deposit(uint _amount, address _token, bool _isETH) public payable {
```

# 送金するETHより小さい値を引数に指定してdeposit出来る

## 要約
ETHの場合、ウォレットから出ていく値とdepositに指定出来る値が異なっていてもエラーにならない

## バグの詳細
1. depositのamountを0.5ethに設定する
2. ウォレットからの送金額を1.0ethに設定する
3. depositを実行
4. コントラクトの残高は1.0eth増える
5. ウォレットの残高は1.0eth減る

## インパクト
depositを行うユーザのethがGOXする可能性がある

## 対象となるコード
```solidity
function deposit(
        uint256 _amount,
        address _token,
        bool _isETH
    ) 
```

## 使用したツール
hardhat

## 改善方法
2つある
1. ETHと他のトークンのdeposit関数を分ける
2. ETHの場合はdeposit時にamountを設定不可とする


# 残高がある時にdepositを呼ぶと資金を失う

## 要約
*前提: depositは残高が0の場合のみ受け付けるという仕様(何度も呼んで残高を増やしていくことは想定外とする)*

以前にdepositを行い、残高がまだある状態で再度depositを行うと、以前の残高は失われ新しい残高で上書きされる

## バグの詳細
1. 9/20に100トークンのデポジットを行う
2. lastTime:9/20, amount:100トークンが記録される
3. 9/30に10トークンのデポジットを行う
4. lastTime:9/20, amount:100トークンが記録は消え、lastTime:9/30, amount:10トークンの記録だけが残る

## インパクト
ユーザーが資産を失う可能性がある

## 対象となるコード
```solidity
        depositInfo.lastTime = uint40(block.timestamp);
        depositInfo.amount = _amount;
        depositAmt[msg.sender][_token] = depositInfo;
```

## 使用したツール
特になし

## 改善方法
残高が無いもしくは0である場合以外はエラーとする

---

以下hamaupさん記載

# [GTeam 0X] withdraw()預金残高を超えても引き出しができてしまう。

## 要約　

withdraw()で、預金残高を超えても引き出しができてしまう。
引き出しをした際に、預金残高を更新するように提案する。

## バグの詳細

withdraw()を預金残高以下の引き出し額に指定して実行した場合、預金残高を保存している
depositAmt[msg.sender][_token].amountの値が更新されない。

もう一度、withdraw()を実行した際に、また同じ預金残高の値を使用して引き出し処理をしてしまう。

## インパクト

コントラクト残高がなくなるまで引き出しされてしまう。

## 対象となるコード


```solidity=
 
  function withdraw(
    address _to,
    uint _amount,
    bool _isETH,
    address _token
  ) public {
    if (!_isXXX(_token, whitelist)) revert();
    TransferInfo memory info = TransferInfo({
        isETH: _isETH,
        token: _token;
        from: address(this),
        amount: _amount,
        to: _to,
    });
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    canWithdrawAmount = 0;
    _tokenTransfer(_info);
    uint rewardAmount = getReward(_token);
    IERC20(BBBToken).transfer(msg.sender, rewardAmount);
  }
```

## 使用したツール
特になし

## 改善方法

withdraw() 引き出し額を差し引いて預金残高　`depositAmt[msg.sender][_token].amount`　を更新する。

#### before

```solidity=
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    canWithdrawAmount = 0;
```

#### after
```solidity=
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    canWithdrawAmount = 0;
    depositAmt[msg.sender][_token].amount -= _amount
```


# [GTeam 0X] withdraw()引き出し額を0に指定した場合でもrewardを受け取れてしまう。

## 要約

withdraw()引き出し額を0に指定した場合でもrewardを受け取れてしまう。
その場合は、報酬金`reward` を送れないようにする。

## バグの詳細

 少しでも預金残高がある場合、withdraw()で引き出し額　`_amount` を0に指定しても、報酬額`reward` を決定され送信されてしまう。

1. 少額をdeposit()する。
2. withdraw()引き出し額　`_amount` を0に指定する
3. 引き出し額 `_amount` は、転送する額に`info.amount`保持される。
4. 預金残高 `depositAmt[msg.sender][_token].amount`　が1でもある場合、
以下条件パスする。

```solidity=
require(_info.amount < canWithdrawAmount, "ERROR");
```
5. getReward()内で預金残高で報酬金`reward` を決定
6. withdraw()を実行した者`msg.sender`に報酬金`reward` を送信

## インパクト

引き出し額　`_amount` を0に指定して引き出しを繰り返されると用意している報酬額`reward`がすべてなくなってしまう。


## 対象となるコード


```solidity=
 
  function withdraw(
    address _to,
    uint _amount,
    bool _isETH,
    address _token
  ) public {
    if (!_isXXX(_token, whitelist)) revert();
    TransferInfo memory info = TransferInfo({
        isETH: _isETH,
        token: _token;
        from: address(this),
        amount: _amount,
        to: _to,
    });
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    canWithdrawAmount = 0;
    _tokenTransfer(_info);
    uint rewardAmount = getReward(_token);
    IERC20(BBBToken).transfer(msg.sender, rewardAmount);
  }
```

## 使用したツール
特になし

## 改善方法

# [GTeam 0X]の改善方法を併用して、引き出し額 `_amount` が0では報酬金`reward` を送れないようにする。

#### before

```solidity=
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    canWithdrawAmount = 0;
```

#### after
```solidity=
    uint canWithdrawAmount = depositAmt[msg.sender][_token].amount;
    require(_info.amount < canWithdrawAmount, "ERROR");
    require(_info.amount > 0, "ERROR");
    canWithdrawAmount = 0;
    depositAmt[msg.sender][_token].amount -= _amount
```

[GTeam 03] 預け入れから引き出しまでの期間が長いほど報酬金`reward`が少なくなる。
## 要約

預け入れから引き出しまでの期間が長いほど報酬金`reward`が少なくなってしまい、即座に預け入れ引き出しを繰り返す利用方法が増えてしまう。

報酬金`reward`の計算方法とdeposit()のlasttimeの値の保存方法の検討を提案する。

## バグの詳細

報酬金`reward`計算式です。

```solidity=
    reward = (REWARD_RATE / (block.timestamp - lastTime)) * amount;
```

1. 初めてdeposit()を実行した時に、lastTimeにタイムスタンプが入ります。

deposit()
```solidity=
    depositInfo.lastTime = uint40(block.timestamp);
```

2. widthdraw()で報奨金計算 getReward（）を呼び出した時に、lastTimeにタイムスタンプが入ります。

getReward（）
```solidity=
    uint lastTime = depositAmt[msg.sender][token].lastTime;
```

3. 報奨金が計算されます。その際、widthdraw()で報奨金計算した時から初めてdeposit()を実行した時の差分が分母に入ります。

getReward（）
```solidity=
    reward = (REWARD_RATE / (block.timestamp - lastTime)) * amount;
```

例えば、初めてdeposit()を実行した後に、もう一度deposit()をした場合、lastTimeが2回目に実行したタイムスタンプに置き換えられる。

deposit()
```solidity=
    uint lastTime = depositAmt[msg.sender][token].lastTime;
```

その場合、現在のタイムスタンプから初めてdeposit()を実行した期間より現在のタイムスタンプから2回目にdeposit()を実行した期間の方が短い、つまり報酬金計算式の分母が小さくなるため、預け入れから引き出しまでの期間が長いほど報酬金`reward`が少なくなってしまう。

また報酬金計算式内にblock.timestampを使用しているので、誤差が出る可能性があり、不正なマイナーによる改ざんもありセキュリティ面で安全ではない。

deposit()のlasttimeの値は、預け入れする度に上書きされてしまう。引き出しを行わず、預け入れを繰り返し行った場合、最後の預け入れをした時のタイムスタンプに置き換わってしまう。

## インパクト

短く預け引き出す方が報酬が高くなってしまうため、即座に預け入れ引き出しを繰り返す利用方法が増えてしまう。

## 対象となるコード

```solidity=
  function getReward(address token) public view returns (uint reward) {
    uint amount = depositAmt[msg.sender][token].amount;
    uint lastTime = depositAmt[msg.sender][token].lastTime;
    reward = (REWARD_RATE / (block.timestamp - lastTime)) * amount;
  }
```

## 使用したツール
特になし

## 改善方法

報酬金`reward`の計算方法とdeposit()のlasttimeへの値の保存方法の検討を提案する。