# BBB
## 概要
- TokenをDepositした分、RewardをもらえるDappsです。
- Depositしている時間が長いほど多くの報酬をもらえる。
- 報酬のトークンはBBB。
# Bug
## [Cardene-H1] 同じユーザーが同じトークンを追加できない。
<details>
## [Cardene-H1] 同じユーザーが同じトークンを追加した際上書きされてしまう。
## 要約
- 同じユーザーが同じトークンを追加するために`deposit()`した際、`depositAmt`が更新されるのではなく上書きされてしまう。
## バグの詳細
- ユーザーAがトークンBを指定してdepositし、その後追加でトークンBを指定してdepositした際、既存のデータに追加されるのではなく、データが上書きされてしまう。
- これにより2回以上同じトークンを指定して`deposit()`を実行したユーザーは`widthdraw()`を呼び出したときに、最後にdepositしたデータを参照してトークン or Ethを受け取ることになり、それ以前に預けた資金が失われる。
## インパクト
- 2回以上同じトークンをdepositしたユーザーは一部の資金を失うことになる。
## 対象となるコード
```solidity=
function deposit(uint _amount, address _token, bool _isETH) public {
...
depositAmt[msg.sender][_token] = depositInfo;
}
```
## 使用したツール
特になし
## 改善方法
- 既に同じユーザーで同じトークンが追加されているかのmappingを追加し、追加されていれば既に追加されている値と足し合わせる。
- before
```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;
}
```
- after
```solidity=
// msg.sednerがdepositしたtokenを既に追加されているdepositAmtに追加されているか。
mapping(address => mapping(address => bool)) isDeposit;
...
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);
// 既にユーザーが同じトークンを追加しているかチェック。
if (depositAmt[msg.sender][_token].from == address(0) && isDeposit[msg.sender][_token] == false) {
// 追加されていない時の処理
depositInfo.amount = _amount;
depositAmt[msg.sender][_token] = depositInfo;
isDeposit[msg.sender][_token] = true;
} else {
// 追加されていた時の処理
depositInfo.amount = depositAmt[msg.sender][_token].amount + _amount;
depositAmt[msg.sender][_token] = depositInfo;
}
}
...
// トークンを引き出した際、isDepositをfalseにする。
function withdraw(
address _to,
uint _amount,
bool _isETH,
address _token
) public {
...
isDeposit[msg.sender][_token] = false;
```
</details>
## [Cardene-H2] `widthdraw()`実行後に`depositAmt`が更新されずReentrancy Attackでによりトークンが全て抜かれてしまう。
<details>
## 要約
- `widthdraw()`を実行してBBBトークンを転送する際、`depositAmt`の更新が行われておらず、悪意あるユーザーにReentrancy Attackでトークンを全て抜かれてしまう。
## バグの詳細
- `widthdraw()`を実行すると、預けた資金に応じて受け取れるBBBトークンを計算して取得できる。
- しかし、BBBトークンを送金したのち、`depositAmt`を更新していないため、再度`widthdraw()`を実行してもBBBトークンを受け取ることができる。
- 一度`deposit()`を実行したユーザーであれば、何度でも`widthdraw()`を実行できてしまうため、BBBトークンが全て抜かれてしまう。
## インパクト
- BBBトークンが全て抜かれてしまう。
## 対象となるコード
- https://gist.github.com/Tomosuke0930/5624c8fde570aae10e2cec00c4c2758a#file-unsecure-sol-L99-L119
```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);
}
```
## 使用したツール
特になし
## 改善方法
- `widthdraw()`を実行し、BBBトークンを送金する前に`depositAmt`を更新する。
- before
```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);
}
```
- after
```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);
// depositAmtの更新。
depositAmt[msg.sender][_token].amount = 0;
IERC20(BBBToken).transfer(msg.sender, rewardAmount);
}
```
</details>
## [Cardene-M1] `whitelist`にいくらでも追加できてしまい、チェック時にガス代不足でチェックが失敗する可能性。
<details>
## 要約
`addWhitelist()`を実行すると`whitelist`配列に要素が追加されるが、既に追加されている要素も再度追加されてしまうため無限に要素を追加できてしまい、チェック時にガス代の不足により処理が失敗されてしまう。
## バグの詳細
- `addWhitelist()`を実行すると`whitelist`配列に要素が追加されるが、既に追加されている要素も再度追加されてしまうため無限に要素を追加できてしまう。
- https://gist.github.com/Tomosuke0930/5624c8fde570aae10e2cec00c4c2758a#file-unsecure-sol-L77-L80
```solidity=
function addWhitelist(address _token) public {
if (!_isXXX(_token, approvedTokens)) revert();
whitelist.push(_token);
}
```
- 攻撃者によって何度も`addWhitelist()`を実行され、`whitelist`に大量のデータが追加されるたとき、`whitelist`に特定のトークンが含まれているか確認する際にガス代が不足して処理が中断されてしまう。
- `deposit()`や`withdraw()`で`!_isXXX(_token, whitelist)`を実行した際。
## インパクト
- 大量のデータをfor文で回すことでガス代が不足して処理が中断されてしまい、`deposit()`や`withdraw()`が一生通らなくなってしまう。
## 対象となるコード
- https://gist.github.com/Tomosuke0930/5624c8fde570aae10e2cec00c4c2758a#file-unsecure-sol-L77-L80
```solidity=
function addWhitelist(address _token) public {
if (!_isXXX(_token, approvedTokens)) revert();
whitelist.push(_token);
}
```
## 使用したツール
特になし
## 改善方法
`addWhitelist()`に`whitetlist`にもトークンが既に追加されていないかチェックする。
- before
```solidity=
function addWhitelist(address _token) public {
if (!_isXXX(_token, approvedTokens)) revert();
whitelist.push(_token);
}
```
- after
```solidity=
function addWhitelist(address _token) public {
if (!_isXXX(_token, approvedTokens) && !_isXXX(_token, whitelist)) revert();
whitelist.push(_token);
}
```
</details>
## [Cardene-M2] `deposit()`に`payable`修飾子と`receive()`、`fallback()`がない。
<details>
## 要約
etherを受け取る`deposit()`に`payable`修飾子と`receive()`、`fallback()`がないため、実行が失敗する。
## バグの詳細
- etherを受け取る`deposit()`に`payable`修飾子と`receive()`、`fallback()`の記載がない。
- `payable`修飾子はコントラクトのアドレスにEthを送金するために必要。
- https://nimil.jp/blog/3way-to-receive-eth
- `receive()`がないためEthを受け取ることができない。
- https://note.com/standenglish/n/n24bbfdb27199
- https://zenn.dev/maztak/articles/933c42e9387602
- `receive()`がなく、`fallback()`もないためエラーがないためEthをコントラクトアドレスに追加できずエラーになってしまう。
- https://www.geeksforgeeks.org/solidity-fall-back-function/
> The function below is a fallback function. It is declared payable which allows it to accept transfer value. It is called in two cases
> A contract receives only Ether and no data.
> No function calls matched even though the account received data.
> This helps us to protect the function from throwing an error.
> 以下の関数は、フォールバック関数です。これは、それが転送値を受け入れることができますpayable宣言されています。この関数は次の2つのケースで呼び出されます。
> コントラクトがEtherのみを受け取り、データを受け取らない場合。
> アカウントがデータを受け取ったにもかかわらず、マッチした関数が呼び出されない。
> これは、関数がエラーを投げるのを防ぐのに役立ちます。
- https://note.com/standenglish/n/n24bbfdb27199
## インパクト
- Ethをコントラクトアドレスに追加できずエラーになってしまう。
##
- https://gist.github.com/Tomosuke0930/5624c8fde570aae10e2cec00c4c2758a#file-unsecure-sol-L82-L97
```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;
}
```
## 使用したツール
特になし
## 改善方法
- `deposit()`に`payable`修飾子と`receive()`、`fallback()`を追加。
- before
```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;
}
```
- after
```solidity=
// receiveの追加
receive() external payable {
...
}
// fallbackの追加
fallback() external payable {
...
}
// payableの追加
function deposit(uint _amount, address _token, bool _isETH) public payable {
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;
}
```
</details>
## [Cardene-M3] `addApprovedTokens()`を呼び出すことができない。
<details>
## 要約
`addApprovedTokens()`を呼び出して許可するコインを追加するが、`addApprovedTokens()`が`private`のため呼び出すことができない。
## バグの詳細
- `approvedTokens`にはownerが許可したコインのアドレスを格納します。
- `addApprovedTokens()`を実行することでownerは許可するコインのアドレスを追加できる。
- しかし、`addApprovedTokens()`が`private`関数のため、誰も呼び出せなくなってしまっている。
- そのため、`addWhitelist()`のチェックが通らなくなり`whitelist`に何も追加できなくなってしまう。
- `whitelist`に何も追加できないと`deposit()`と`whithdraw()`の実行が通らなくなり、誰も資金を預けたり引き出したりできない。
## インパクト
- `addApprovedTokens()`を実行できなくなり、誰も資金を預けることができなくなる。
## 対象となるコード
```solidity=
function addApprovedTokens(address _token) private {
if (msg.sender != owner) revert();
approvedTokens.push(_token);
}
```
## 使用したツール
特になし
## 改善方法
- `private`ではなく、`public`にする。
- before
```solidity=
function addApprovedTokens(address _token) private {
if (msg.sender != owner) revert();
approvedTokens.push(_token);
}
```
- after
```solidity=
// publicに変更
function addApprovedTokens(address _token) public {
if (msg.sender != owner) revert();
approvedTokens.push(_token);
}
```
</details>
## [Cardene-]
<details>
## 要約
## バグの詳細
## インパクト
## 対象となるコード
## 使用したツール
特になし
## 改善方法
</details>
Sign in with Wallet
Connect another wallet