-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
a81f9e5
commit 9248e64
Showing
735 changed files
with
56,673 additions
and
10 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
Colossal Chiffon Urchin | ||
|
||
Medium | ||
|
||
# authorProfileId can avoid being slashed | ||
|
||
### Summary | ||
|
||
there is not lock lock on lock on staking (and withdrawals) for the accused authorProfileId | ||
|
||
### Root Cause | ||
|
||
According to [docs](https://whitepaper.ethos.network/ethos-mechanisms/slash) their should be lock | ||
> Any Ethos participant may act as a "whistleblower" to accuse another participant of inaccurate claims or unethical behavior. This accusation triggers a 24h lock on staking (and withdrawals) for the accused. | ||
Currently anyone can unvouch at any time | ||
```solidity | ||
function unvouch(uint256 vouchId) public whenNotPaused nonReentrant { | ||
Vouch storage v = vouches[vouchId]; | ||
_vouchShouldExist(vouchId); | ||
_vouchShouldBePossibleUnvouch(vouchId); | ||
// because it's $$$, you can only withdraw/unvouch to the same address you used to vouch | ||
// however, we don't care about the status of the address's profile; funds are always attached | ||
// to an address, not a profile | ||
if (vouches[vouchId].authorAddress != msg.sender) { | ||
revert AddressNotVouchAuthor(vouchId, msg.sender, vouches[vouchId].authorAddress); | ||
} | ||
v.archived = true; | ||
// solhint-disable-next-line not-rely-on-time | ||
v.activityCheckpoints.unvouchedAt = block.timestamp; | ||
// remove the vouch from the tracking arrays and index mappings | ||
_removeVouchFromArrays(v); | ||
// apply fees and determine how much is left to send back to the author | ||
(uint256 toWithdraw, ) = applyFees(v.balance, false, v.subjectProfileId); | ||
// set the balance to 0 and save back to storage | ||
v.balance = 0; | ||
// send the funds to the author | ||
// note: it sends it to the same address that vouched; not the one that called unvouch | ||
(bool success, ) = payable(v.authorAddress).call{ value: toWithdraw }(""); | ||
if (!success) { | ||
revert FeeTransferFailed("Failed to send ETH to author"); | ||
} | ||
emit Unvouched(v.vouchId, v.authorProfileId, v.subjectProfileId); | ||
} | ||
``` | ||
[contracts/contracts/EthosVouch.sol#L452](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L452) | ||
### Internal pre-conditions | ||
|
||
_No response_ | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
Accused profile sees that a lot of complains going against him and unvouch all vouched funds before slashing | ||
|
||
### Impact | ||
|
||
authorProfileId can avoid being slashed | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
```diff | ||
|
||
+ function pauseActions(uint authorProfileId) external onlyOwner{ | ||
+ ... | ||
+ } | ||
|
||
function unvouch(uint256 vouchId) public whenNotPaused nonReentrant { | ||
+ uint256 authorProfileId = IEthosProfile( | ||
+ contractAddressManager.getContractAddressForName(ETHOS_PROFILE) | ||
+ ).verifiedProfileIdForAddress(msg.sender); | ||
+ require(!isActionsPaused(authorProfileId), "actions paused") | ||
Vouch storage v = vouches[vouchId]; | ||
_vouchShouldExist(vouchId); | ||
_vouchShouldBePossibleUnvouch(vouchId); | ||
|
||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
Tricky Sage Stallion | ||
|
||
Medium | ||
|
||
# Users can increase vouch when EthosVouch is paused | ||
|
||
### Summary | ||
|
||
The [`EthosVouch::increaseVouch()`](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L426) lacks the `whenNotPaused` modifier. This allows users to increase for an existing vouch even when the contract is paused. | ||
|
||
|
||
|
||
### Root Cause | ||
|
||
The [`EthosVouch::increaseVouch()`](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L426) lacks the `whenNotPaused` modifier. | ||
|
||
When the `EthosVouch` contract is paused, no other user-access functions can be executed. | ||
|
||
### Internal pre-conditions | ||
|
||
- The `EthosVouch` contract is in a paused state. | ||
- A user has vouched for an address. | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
This issue allows unauthorized vouching during a paused state. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Add the `whenNotPaused` modifier to the `increaseVouch()` function. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
Colossal Chiffon Urchin | ||
|
||
Medium | ||
|
||
# slashing will be happening without gracing period | ||
|
||
### Summary | ||
|
||
Actors will be able to constantly empty someone's account in a short time | ||
|
||
### Root Cause | ||
|
||
According [to docs](https://whitepaper.ethos.network/ethos-mechanisms/slash) their should be a grace period between slashing, there is none, right now | ||
> Upon being slashed the accused has a 72h grace period before they may be slashed again. | ||
```solidity | ||
function slash( | ||
uint256 authorProfileId, | ||
uint256 slashBasisPoints | ||
) external onlySlasher whenNotPaused nonReentrant returns (uint256) { | ||
if (slashBasisPoints > MAX_SLASH_PERCENTAGE) { | ||
revert InvalidSlashPercentage(); | ||
} | ||
uint256 totalSlashed; | ||
uint256[] storage vouchIds = vouchIdsByAuthor[authorProfileId]; | ||
for (uint256 i = 0; i < vouchIds.length; i++) { | ||
Vouch storage vouch = vouches[vouchIds[i]]; | ||
// Only slash active vouches | ||
if (!vouch.archived) { | ||
uint256 slashAmount = vouch.balance.mulDiv( | ||
slashBasisPoints, | ||
BASIS_POINT_SCALE, | ||
Math.Rounding.Floor | ||
); | ||
if (slashAmount > 0) { | ||
vouch.balance -= slashAmount; | ||
totalSlashed += slashAmount; | ||
} | ||
} | ||
} | ||
if (totalSlashed > 0) { | ||
// Send slashed funds to protocol fee address | ||
(bool success, ) = protocolFeeAddress.call{ value: totalSlashed }(""); | ||
if (!success) revert FeeTransferFailed("Slash transfer failed"); | ||
} | ||
emit Slashed(authorProfileId, slashBasisPoints, totalSlashed); | ||
return totalSlashed; | ||
} | ||
``` | ||
[contracts/contracts/EthosVouch.sol#L520](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L520) | ||
|
||
### Internal pre-conditions | ||
|
||
_No response_ | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
Seems like their will be permissionless accusing of account, so someone's account could be emptied in a short time | ||
|
||
### Impact | ||
|
||
Someone's account can be slashed multiple times in a short time | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
tracked slashed time, implement grace period |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
Tricky Sage Stallion | ||
|
||
Medium | ||
|
||
# Voucher can avoid slashing penalties by front-running with `unvouch()` | ||
|
||
### Summary | ||
|
||
The [`EthosVouch::slash()`](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L520) slashes up to 10% of all vouch balances for a given voucher. However, the voucher can front-run the slashing transaction by calling [`unvouch()`](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L452) and paying the exit fee. If the exit fee is significantly lower than the penalties, the voucher can minimize or avoid most of the losses. | ||
|
||
|
||
### Root Cause | ||
|
||
The current implementation of `slash()` does not account for the possibility of a voucher executing an `unvouch()` transaction immediately before the slashing is finalized. | ||
|
||
|
||
### Internal pre-conditions | ||
|
||
- The `slash()` function is called to penalize a voucher. | ||
- The voucher has sufficient time to detect the slashing transaction and front-run it by calling `unvouch()`. | ||
|
||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
Allowing malicious vouchers to avoid penalties. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
One possible option could be implementing an unvouch queue, unvouch requests can still be slashed. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
Colossal Chiffon Urchin | ||
|
||
Medium | ||
|
||
# User can vouch for an archived profile | ||
|
||
### Summary | ||
increaseVouch allows to increase power of archived account. | ||
### Root Cause | ||
According to docs in code | ||
> - Author cannot vouch for an archived profile | ||
[EthosVouch.sol#L32](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L32) | ||
|
||
inside `vouchByProfileId` there is a check that subject's profile is not archived, but there is none in increaseVouch, which allows to first create vouch-> archive account->increase account's power in archived state, which violates invariant in the beginning of contract | ||
```solidity | ||
function vouchByProfileId( | ||
uint256 subjectProfileId, | ||
string calldata comment, | ||
string calldata metadata | ||
) public payable whenNotPaused nonReentrant { | ||
... | ||
(bool verified, bool archived, bool mock) = IEthosProfile( | ||
contractAddressManager.getContractAddressForName(ETHOS_PROFILE) | ||
).profileStatusById(subjectProfileId); | ||
// you may not vouch for archived profiles | ||
// however, you may vouch for verified AND mock profiles | ||
// we allow vouching for mock profiles in case they are later verified | ||
if (archived || (!mock && !verified)) { | ||
revert InvalidEthosProfileForVouch(subjectProfileId); | ||
} | ||
... | ||
} | ||
``` | ||
[contracts/EthosVouch.sol#L330](https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/EthosVouch.sol#L330) | ||
### Internal pre-conditions | ||
|
||
_No response_ | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
Archived account's power can be increased which breaks protocol's invariant | ||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
Send it to the subject reward escrow like its done with dust at the end of the function | ||
```diff | ||
function increaseVouch(uint256 vouchId) public payable nonReentrant { | ||
// vouch increases much also meet the minimum vouch amount | ||
if (msg.value < configuredMinimumVouchAmount) { | ||
revert MinimumVouchAmount(configuredMinimumVouchAmount); | ||
} | ||
+ (bool verified, bool archived, bool mock) = IEthosProfile( | ||
+ contractAddressManager.getContractAddressForName(ETHOS_PROFILE) | ||
+ ).profileStatusById(vouches[vouchId].subjectProfileId); | ||
+ | ||
+ // you may not vouch for archived profiles | ||
+ // however, you may vouch for verified AND mock profiles | ||
+ // we allow vouching for mock profiles in case they are later verified | ||
+ if (archived || (!mock && !verified)) { | ||
+ revert InvalidEthosProfileForVouch(vouches[vouchId].subjectProfileId); | ||
+ } | ||
|
||
// get the profile id of the author | ||
uint256 profileId = IEthosProfile( | ||
contractAddressManager.getContractAddressForName(ETHOS_PROFILE) | ||
).verifiedProfileIdForAddress(msg.sender); | ||
_vouchShouldBelongToAuthor(vouchId, profileId); | ||
// make sure this vouch is active; not unvouched | ||
_vouchShouldBePossibleUnvouch(vouchId); | ||
|
||
uint256 subjectProfileId = vouches[vouchId].subjectProfileId; | ||
(uint256 toDeposit, ) = applyFees(msg.value, true, subjectProfileId); | ||
vouches[vouchId].balance += toDeposit; | ||
|
||
emit VouchIncreased(vouchId, profileId, subjectProfileId, msg.value); | ||
} | ||
``` |
Oops, something went wrong.