Skip to content

Commit

Permalink
semgrep rules
Browse files Browse the repository at this point in the history
  • Loading branch information
bxmmm1 committed Oct 25, 2024
1 parent 0decf42 commit c938f2c
Show file tree
Hide file tree
Showing 23 changed files with 31 additions and 335 deletions.
6 changes: 6 additions & 0 deletions .semgrepignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
*/test/*
*/script/*
semgrep-rules/*
PufToken.sol
GuardianModule.sol
EnclaveVerifier.sol
2 changes: 1 addition & 1 deletion l2-contracts/src/L2RewardManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract L2RewardManager is
* @dev Restricted in this context is like the `whenNotPaused` modifier from Pausable.sol
*/
function claimRewards(ClaimOrder[] calldata claimOrders) external restricted {
for (uint256 i = 0; i < claimOrders.length; i++) {
for (uint256 i = 0; i < claimOrders.length; ++i) {
if (isClaimed(claimOrders[i].intervalId, claimOrders[i].account)) {
revert AlreadyClaimed(claimOrders[i].intervalId, claimOrders[i].account);
}
Expand Down
1 change: 0 additions & 1 deletion mainnet-contracts/.semgrepignore

This file was deleted.

3 changes: 3 additions & 0 deletions mainnet-contracts/src/PufLocker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,10 @@ contract PufLocker is IPufLocker, AccessManagedUpgradeable, UUPSUpgradeable, Puf
uint128 totalAmount = 0;
Deposit[] storage userDeposits = $.deposits[msg.sender][token];

// nosemgrep array-length-outside-loop
for (uint256 i = 0; i < depositIndexes.length; ++i) {
uint256 index = depositIndexes[i];
// nosemgrep array-length-outside-loop
if (index >= userDeposits.length) {
revert InvalidDepositIndex();
}
Expand Down Expand Up @@ -161,6 +163,7 @@ contract PufLocker is IPufLocker, AccessManagedUpgradeable, UUPSUpgradeable, Puf
}

uint256 end = start + limit > totalDeposits ? totalDeposits : start + limit;
// nosemgrep basic-arithmetic-underflow
uint256 count = end - start;

depositPage = new Deposit[](count);
Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferDepositor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ contract PufferDepositor is IPufferDepositor, PufferDepositorStorage, AccessMana
}

// PUFFER_VAULT.deposit will revert if we get no stETH from this contract
// nosemgrep arbitrary-low-level-call
(bool success, bytes memory returnData) = _1INCH_ROUTER.call{ value: msg.value }(callData);
if (!success) {
revert SwapFailed(address(tokenIn), amountIn);
Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferDepositorV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ contract PufferDepositorV2 is IPufferDepositorV2, PufferDepositorStorage, Access
/**
* @notice Returns the pufETH sent to this contract by mistake
*/
// nosemgrep tin-unprotected-initialize
function initialize() public reinitializer(2) {
// https://etherscan.io/token/0xd9a442856c234a39a81a089c06451ebaa4306a72?a=0x4aa799c5dfc01ee7d790e3bf1a7c2257ce1dceff
// slither-disable-next-line unchecked-transfer
Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ contract PufferModule is IPufferModule, Initializable, AccessManagedUpgradeable
returns (bool success, bytes memory)
{
// slither-disable-next-line arbitrary-send-eth
// nosemgrep arbitrary-low-level-call
return to.call{ value: amount }(data);
}

Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ contract PufferModuleManager is IPufferModuleManager, AccessManagedUpgradeable,
uint256 sharesWithdrawn;

for (uint256 i = 0; i < withdrawals.length; ++i) {
// nosemgrep array-length-outside-loop
for (uint256 j = 0; j < withdrawals[i].shares.length; ++j) {
sharesWithdrawn += withdrawals[i].shares[j];
}
Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferOracleV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ contract PufferOracleV2 is IPufferOracleV2, AccessManaged {
* @dev Restricted to PufferProtocol contract
*/
function exitValidators(uint256 numberOfExits) public restricted {
// nosemgrep basic-arithmetic-underflow
_numberOfActivePufferValidators -= numberOfExits;
emit NumberOfActiveValidators(_numberOfActivePufferValidators);
}
Expand Down
4 changes: 4 additions & 0 deletions mainnet-contracts/src/PufferProtocol.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
}

// Reverts if insufficient balance
// nosemgrep basic-arithmetic-underflow
$.nodeOperatorInfo[msg.sender].vtBalance -= amount;

// slither-disable-next-line unchecked-transfer
Expand Down Expand Up @@ -329,6 +330,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
burnAmounts.vt += vtBurnAmount;

// Store the withdrawal amount for that node operator
// nosemgrep basic-arithmetic-underflow
bondWithdrawals[i].pufETHAmount = (bondAmount - burnAmount);

emit ValidatorExited({
Expand All @@ -342,6 +344,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
// Decrease the number of registered validators for that module
_decreaseNumberOfRegisteredValidators($, validatorInfos[i].moduleName);
// Storage VT and the active validator count update for the Node Operator
// nosemgrep basic-arithmetic-underflow
$.nodeOperatorInfo[validator.node].vtBalance -= SafeCast.toUint96(vtBurnAmount);
--$.nodeOperatorInfo[validator.node].activeValidatorCount;

Expand Down Expand Up @@ -401,6 +404,7 @@ contract PufferProtocol is IPufferProtocol, AccessManagedUpgradeable, UUPSUpgrad
uint256 vtPenalty = $.vtPenalty;
// Burn VT penalty amount from the Node Operator
VALIDATOR_TICKET.burn(vtPenalty);
// nosemgrep basic-arithmetic-underflow
$.nodeOperatorInfo[node].vtBalance -= SafeCast.toUint96(vtPenalty);
--$.nodeOperatorInfo[node].pendingValidatorCount;

Expand Down
2 changes: 1 addition & 1 deletion mainnet-contracts/src/PufferRevenueDepositor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ contract PufferRevenueDepositor is
if (address(this).balance > 0) {
WETH.deposit{ value: address(this).balance }();
}

// nosemgrep tin-reentrant-dbl-diff-balance
uint256 rewardsAmount = WETH.balanceOf(address(this));

uint256 treasuryRewards = (rewardsAmount * $.treasuryRewardsBps) / _BASIS_POINT_SCALE;
Expand Down
2 changes: 2 additions & 0 deletions mainnet-contracts/src/PufferVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ contract PufferVault is
revert InvalidWithdrawal();
}

// nosemgrep basic-arithmetic-underflow
$.eigenLayerPendingWithdrawalSharesAmount -= queuedWithdrawal.shares[0];

_EIGEN_STRATEGY_MANAGER.completeQueuedWithdrawal({
Expand Down Expand Up @@ -274,6 +275,7 @@ contract PufferVault is
SafeERC20.safeIncreaseAllowance(_ST_ETH, address(_LIDO_WITHDRAWAL_QUEUE), lockedAmount);
requestIds = _LIDO_WITHDRAWAL_QUEUE.requestWithdrawals(amounts, address(this));

// nosemgrep array-length-outside-loop
for (uint256 i = 0; i < requestIds.length; ++i) {
$.lidoWithdrawals.add(requestIds[i]);
}
Expand Down
6 changes: 6 additions & 0 deletions mainnet-contracts/src/PufferVaultV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
/**
* @notice Changes underlying asset from stETH to WETH
*/
// nosemgrep tin-unprotected-initialize
function initialize() public reinitializer(2) {
// In this initialization, we swap out the underlying stETH with WETH
ERC4626Storage storage erc4626Storage = _getERC4626StorageInternal();
Expand Down Expand Up @@ -299,6 +300,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
SafeERC20.safeIncreaseAllowance(_ST_ETH, address(_LIDO_WITHDRAWAL_QUEUE), lockedAmount);
requestIds = _LIDO_WITHDRAWAL_QUEUE.requestWithdrawals(amounts, address(this));

// nosemgrep array-length-outside-loop
for (uint256 i = 0; i < requestIds.length; ++i) {
$.lidoWithdrawalAmounts.set(requestIds[i], amounts[i]);
}
Expand Down Expand Up @@ -333,6 +335,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
uint256 balanceAfter = address(this).balance;
uint256 actualWithdrawal = balanceAfter - balanceBefore;
// Deduct from the locked amount the expected amount
// nosemgrep basic-arithmetic-underflow
$.lidoLockedETH -= expectedWithdrawal;

emit ClaimedWithdrawals(requestIds);
Expand All @@ -352,6 +355,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
uint256 ethBalance = address(this).balance;
if (ethBalance < ethAmount) {
// Reverts if no WETH to unwrap
// nosemgrep basic-arithmetic-underflow
_WETH.withdraw(ethAmount - ethBalance);
}

Expand Down Expand Up @@ -456,6 +460,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
*/
function previewRedeem(uint256 shares) public view virtual override returns (uint256) {
uint256 assets = super.previewRedeem(shares);
// nosemgrep basic-arithmetic-underflow
return assets - _feeOnTotal(assets, getExitFeeBasisPoints());
}

Expand Down Expand Up @@ -524,6 +529,7 @@ contract PufferVaultV2 is PufferVault, IPufferVaultV2 {
revert InvalidWithdrawal();
}

// nosemgrep
$.eigenLayerPendingWithdrawalSharesAmount -= queuedWithdrawal.shares[0];

_DELEGATION_MANAGER.completeQueuedWithdrawal({
Expand Down
1 change: 1 addition & 0 deletions mainnet-contracts/src/PufferVaultV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ contract PufferVaultV3 is PufferVaultV2, IPufferVaultV3 {
VaultStorage storage $ = _getPufferVaultStorage();

uint256 previousMintAmount = $.totalRewardMintAmount;
// nosemgrep basic-arithmetic-underflow
uint256 newMintAmount = previousMintAmount - ethAmount;
$.totalRewardMintAmount = newMintAmount;

Expand Down
2 changes: 2 additions & 0 deletions mainnet-contracts/src/PufferWithdrawalManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ contract PufferWithdrawalManager is
expectedETHAmount: expectedETHAmount
});

// nosemgrep basic-arithmetic-underflow
uint256 diff = transferAmount - batch.amountClaimed;
totalExcessETH += diff;

Expand All @@ -256,6 +257,7 @@ contract PufferWithdrawalManager is
}

if (totalExcessETH > 0) {
// nosemgrep arbitrary-low-level-call
(bool success,) = address(PUFFER_VAULT).call{ value: totalExcessETH }("");
require(success, TransferFailed());

Expand Down
15 changes: 0 additions & 15 deletions semgrep-rules/best-practice/use-ownable2step.sol

This file was deleted.

25 changes: 0 additions & 25 deletions semgrep-rules/best-practice/use-ownable2step.yaml

This file was deleted.

85 changes: 0 additions & 85 deletions semgrep-rules/performance/state-variable-read-in-a-loop.sol

This file was deleted.

54 changes: 0 additions & 54 deletions semgrep-rules/performance/state-variable-read-in-a-loop.yaml

This file was deleted.

Loading

0 comments on commit c938f2c

Please sign in to comment.