Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Guard against malicious processors [SUP-8873] #639

Merged
merged 54 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
5b5d429
fix: add RECEIVER_ADDRESS_EQUAL_TO_PROCESSOR_ADDRESS() error
TamaraRingas Oct 16, 2024
11cc0da
fix: move RECEIVER_ADDRESS_EQUAL_TO_PROCESSOR_ADDRESS() error to ISup…
TamaraRingas Oct 16, 2024
f380c7a
chore: rename IS_ROUTER_PLUS_PROCESSOR() error
TamaraRingas Oct 16, 2024
00151da
chore: rm unused error
TamaraRingas Oct 16, 2024
71d5769
add NOT_CORE_STATE_REGISTRY_RESCUER() error
TamaraRingas Oct 16, 2024
5f472a0
add REFUND_AMOUNT_EXCEEDS_RECEIVED_AMOUNT() error
TamaraRingas Oct 16, 2024
5fb5af1
chore: rename REFUND_AMOUNT_EXCEEDS_EXPECTED_AMOUNT() error
TamaraRingas Oct 16, 2024
c1c2f33
feat: add requestRefund() function
TamaraRingas Oct 16, 2024
4879f8e
fix: update requestRefund() params
TamaraRingas Oct 16, 2024
53ac164
chore: add onlyCoreStateRegistryRescuer() modifier
TamaraRingas Oct 16, 2024
a0a746e
feat: add approveRefund()
TamaraRingas Oct 16, 2024
a79275c
refactor: rm old refund functions
TamaraRingas Oct 16, 2024
9924cab
refactor: rm old refund functions
TamaraRingas Oct 16, 2024
e42992c
refactor: rm _getDelay()
TamaraRingas Oct 16, 2024
7f790d3
refactor: rm refund proposal time checks
TamaraRingas Oct 16, 2024
26d0e22
refactor: rm tests for old refund mechanism
TamaraRingas Oct 16, 2024
3506d86
chore: rm unused errors
TamaraRingas Oct 16, 2024
2ec1753
add REFUND_ALREADY_APPROVED() error
TamaraRingas Oct 16, 2024
b9cea45
fix: add checks for duplicate approvals
TamaraRingas Oct 16, 2024
a49417f
chore: add test case for duplicate approvals
TamaraRingas Oct 16, 2024
094e8d8
chore: formatting
TamaraRingas Oct 16, 2024
7ead146
style: formatting
TamaraRingas Oct 17, 2024
fcd9edf
Revert "chore: formatting"
TamaraRingas Oct 17, 2024
0efc2ba
Revert "style: formatting"
TamaraRingas Oct 17, 2024
fc6f31c
fix: formatting
0xTimepunk Oct 17, 2024
477fb32
fix: correct interface of async router plus
0xTimepunk Oct 17, 2024
6fda946
refactor: rm proposedTime from Refund struct
TamaraRingas Oct 17, 2024
c861efc
Merge pull request #640 from superform-xyz/joao-fix-formatting-8873
TamaraRingas Oct 17, 2024
4e9e104
fix: delegate prank
0xTimepunk Oct 17, 2024
aa52182
fix: remainder of files
0xTimepunk Oct 17, 2024
b6ce0b7
test: fix staging test
0xTimepunk Oct 17, 2024
81e9c40
Merge remote-tracking branch 'origin/v1.5' into tamara-sup-8873-guard…
0xTimepunk Oct 17, 2024
19878b6
refactor: rename RefundRequested event
TamaraRingas Oct 18, 2024
b89ec5a
refactor: rename RefundRequested event
TamaraRingas Oct 18, 2024
d6541b0
fix: typo
TamaraRingas Oct 18, 2024
62dd458
fix: update tests
TamaraRingas Oct 21, 2024
8d38ebd
refactor: rm amount from requestRefund()
TamaraRingas Oct 21, 2024
b85030d
refactor: rm amount from requestRefund()
TamaraRingas Oct 21, 2024
1779144
Merge branch 'tamara-sup-8873-guard-against-malicious-processors' of …
TamaraRingas Oct 23, 2024
186182d
fix: reorder test logic
TamaraRingas Oct 24, 2024
af8d01b
add check for expectedAmountInterimAsset == 0 in completeCrossCHainRe…
TamaraRingas Oct 24, 2024
a548dba
fix: simplify request amount to refund
0xTimepunk Oct 24, 2024
5c246e4
refactor: update refund mechanism
TamaraRingas Oct 24, 2024
3684d11
test requested refund amount > expected
TamaraRingas Oct 24, 2024
1d1e450
fix: update requested amount too high test
TamaraRingas Oct 24, 2024
c6191c5
chore: update refund tests
TamaraRingas Oct 24, 2024
ff9a43a
fix: update amount setter in requestRefund()
TamaraRingas Oct 24, 2024
7464511
chore: add extra tests for requestRefund()
TamaraRingas Oct 24, 2024
2375ce7
fix: update invalid approver error selector
TamaraRingas Oct 24, 2024
7d73256
chore: rm unused variable
TamaraRingas Oct 24, 2024
70d8021
fix: update test-vvv
TamaraRingas Oct 24, 2024
60dc92d
fix: update test_crossChainRebalance_updateSuperformData_DifferentRec…
TamaraRingas Oct 24, 2024
bde75d9
wip: update test_crossChainRebalance_updateSuperformData_DifferentRec…
TamaraRingas Oct 25, 2024
48e3015
test: fix
0xTimepunk Oct 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions script/forge-scripts/safe/lib/DelegatePrank.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,28 @@ import "forge-std/console.sol";

*/
contract DelegatePrank is CommonBase {
Delegator delegator = makeDelegator();
function makeDelegator() internal returns (Delegator) {
return new Delegator();
}

function delegatePrank(address from, address to, bytes memory cd) public returns (bool success, bytes memory ret) {
bytes memory code = from.code;
vm.etch(from,address(delegator).code);
(success, ret) = from.call(abi.encodeCall(delegator.etchCodeAndDelegateCall,(to,cd,code)));
}
Delegator delegator = makeDelegator();

function makeDelegator() internal returns (Delegator) {
return new Delegator();
}

function delegatePrank(address from, address to, bytes memory cd) public returns (bool success, bytes memory ret) {
bytes memory code = from.code;
vm.etch(from, address(delegator).code);
(success, ret) = from.call(abi.encodeCall(delegator.etchCodeAndDelegateCall, (to, cd, code)));
}
}

contract Delegator is CommonBase {
function etchCodeAndDelegateCall(address dest, bytes memory cd, bytes calldata code) external payable virtual {
vm.etch(address(this),code);
assembly ("memory-safe") {
let result := delegatecall(gas(), dest, add(cd,32), mload(cd), 0, 0)
returndatacopy(0, 0, returndatasize())
switch result
case 0 { revert(0, returndatasize()) }
default { return(0, returndatasize()) }
function etchCodeAndDelegateCall(address dest, bytes memory cd, bytes calldata code) external payable virtual {
vm.etch(address(this), code);
assembly ("memory-safe") {
let result := delegatecall(gas(), dest, add(cd, 32), mload(cd), 0, 0)
returndatacopy(0, 0, returndatasize())
switch result
case 0 { revert(0, returndatasize()) }
default { return(0, returndatasize()) }
}
}
}
}
3 changes: 1 addition & 2 deletions src/BaseForm.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/IE
/// @dev Abstract contract to be inherited by different Form implementations
/// @author Zeropoint Labs
abstract contract BaseForm is IBaseForm, Initializable, ERC165 {

using DataLib for uint256;

//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -254,7 +253,7 @@ abstract contract BaseForm is IBaseForm, Initializable, ERC165 {
}

/// @dev Checks if the Form implementation has the appropriate interface support
/// @param interfaceId_ is the interfaceId to check
/// @param interfaceId_ is the interfaceId to check
function supportsInterface(bytes4 interfaceId_) public view virtual override(ERC165, IERC165) returns (bool) {
return interfaceId_ == type(IBaseForm).interfaceId || super.supportsInterface(interfaceId_);
}
Expand Down
4 changes: 1 addition & 3 deletions src/crosschain-data/extensions/AsyncStateRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,7 @@ contract AsyncStateRegistry is BaseStateRegistry, IAsyncStateRegistry {
}

/// @inheritdoc IAsyncStateRegistry
function getSyncWithdrawTxDataPayload(
uint256 payloadId_
)
function getSyncWithdrawTxDataPayload(uint256 payloadId_)
external
view
returns (SyncWithdrawTxDataPayload memory syncWithdrawTxDataPayload_)
Expand Down
1 change: 0 additions & 1 deletion src/crosschain-data/utils/QuorumManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Error } from "src/libraries/Error.sol";
/// @dev Quorum thresholds using in sending proofs from chain to chain
/// @author ZeroPoint Labs
abstract contract QuorumManager is IQuorumManager {

//////////////////////////////////////////////////////////////
// STATE VARIABLES //
//////////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ library DeBridgeError {

/// @dev if the swap router is invalid
error INVALID_SWAP_ROUTER();
}
}
1 change: 0 additions & 1 deletion src/crosschain-liquidity/socket/SocketOneInchValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { ISocketOneInchImpl } from "src/vendor/socket/ISocketOneInchImpl.sol";
/// @dev Asserts Socket same-chain txData is valid
/// @author Zeropoint Labs
contract SocketOneInchValidator is BridgeValidator {

//////////////////////////////////////////////////////////////
// CONSTRUCTOR //
//////////////////////////////////////////////////////////////
Expand Down
4 changes: 1 addition & 3 deletions src/forms/ERC4626FormImplementation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { IERC4626 } from "openzeppelin-contracts/contracts/interfaces/IERC4626.s
/// @dev Has common ERC4626 internal functions that can be re-used by implementations
/// @author Zeropoint Labs
abstract contract ERC4626FormImplementation is BaseForm, LiquidityHandler {

using SafeERC20 for IERC20;
using SafeERC20 for IERC4626;
using DataLib for uint256;
Expand Down Expand Up @@ -229,7 +228,7 @@ abstract contract ERC4626FormImplementation is BaseForm, LiquidityHandler {
/// superform data
if (
vars.assetDifference * ENTIRE_SLIPPAGE
< singleVaultData_.amount * (ENTIRE_SLIPPAGE - singleVaultData_.maxSlippage)
< singleVaultData_.amount * (ENTIRE_SLIPPAGE - singleVaultData_.maxSlippage)
) {
revert Error.DIRECT_DEPOSIT_SWAP_FAILED();
}
Expand Down Expand Up @@ -271,7 +270,6 @@ abstract contract ERC4626FormImplementation is BaseForm, LiquidityHandler {
}

function _processDirectWithdraw(InitSingleVaultData memory singleVaultData_) internal returns (uint256 assets) {

DirectWithdrawLocalVars memory vars;

/// @dev if there is no txData, on withdraws the receiver is receiverAddress, otherwise it
Expand Down
1 change: 0 additions & 1 deletion src/forms/interfaces/IERC4626Form.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { IERC20 } from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"
/// @dev Interface for ERC4626Form
/// @author Zeropoint Labs
interface IERC4626Form is IERC20 {

//////////////////////////////////////////////////////////////
// EXTERNAL VIEW FUNCTIONS //
//////////////////////////////////////////////////////////////
Expand Down
4 changes: 1 addition & 3 deletions src/interfaces/IAsyncStateRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ interface IAsyncStateRegistry {
/// @notice retrieves the sync withdraw txData payload for a given payload ID
/// @param payloadId_ The ID of the payload
/// @return syncWithdrawTxDataPayload_ for the specified payload ID
function getSyncWithdrawTxDataPayload(
uint256 payloadId_
)
function getSyncWithdrawTxDataPayload(uint256 payloadId_)
external
view
returns (SyncWithdrawTxDataPayload memory syncWithdrawTxDataPayload_);
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IBaseForm.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { IERC4626 } from "openzeppelin-contracts/contracts/interfaces/IERC4626.s
/// @dev Interface for BaseForm
/// @author ZeroPoint Labs
interface IBaseForm is IERC165 {

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IBaseRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import "src/types/DataTypes.sol";
/// @dev Interface for abstract BaseRouter
/// @author Zeropoint Labs
interface IBaseRouter {

//////////////////////////////////////////////////////////////
// EXTERNAL WRITE FUNCTIONS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IBroadcastAmbImplementation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/// @dev Interface for AMBs used in Broadcasting
/// @author ZeroPoint Labs
interface IBroadcastAmbImplementation {

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
12 changes: 6 additions & 6 deletions src/interfaces/ICoreStateRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/// @dev Interface for CoreStateRegistry
/// @author ZeroPoint Labs
interface ICoreStateRegistry {

//////////////////////////////////////////////////////////////
// STRUCTS //
//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -34,10 +33,7 @@ interface ICoreStateRegistry {

/// @dev is emitted when a rescue is proposed for failed deposits in a payload
event RescueProposed(
uint256 indexed payloadId,
uint256[] superformIds,
uint256[] proposedAmount,
uint256 proposedTime
uint256 indexed payloadId, uint256[] superformIds, uint256[] proposedAmount, uint256 proposedTime
);

/// @dev is emitted when an user disputed his refund amounts
Expand All @@ -64,7 +60,11 @@ interface ICoreStateRegistry {
/// @param finalAmount_ is the final amount of tokens received
/// @param amount_ is the indicated amount of tokens to be received
/// @param maxSlippage_ is the amount of acceptable slippage for the transaction
function validateSlippage(uint256 finalAmount_, uint256 amount_, uint256 maxSlippage_)
function validateSlippage(
uint256 finalAmount_,
uint256 amount_,
uint256 maxSlippage_
)
external
view
returns (bool);
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IDstSwapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/// @dev Interface for DstSwapper
/// @author Zeropoint Labs
interface IDstSwapper {

//////////////////////////////////////////////////////////////
// STRUCTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IEmergencyQueue.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { InitSingleVaultData } from "src/types/DataTypes.sol";
/// @dev Interface for EmergencyQueue
/// @author ZeroPoint Labs
interface IEmergencyQueue {

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IPayMaster.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { LiqRequest } from "src/types/DataTypes.sol";
/// @dev Interface for PayMaster
/// @author ZeroPoint Labs
interface IPayMaster {

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/IQuorumManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/// @dev Interface for QuorumManager
/// @author ZeroPoint Labs
interface IQuorumManager {

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
3 changes: 1 addition & 2 deletions src/interfaces/ISuperPositions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { AMBMessage } from "../types/DataTypes.sol";
/// @dev Interface for SuperPositions
/// @author Zeropoint Labs
interface ISuperPositions is IERC1155A {

//////////////////////////////////////////////////////////////
// STRUCTS //
//////////////////////////////////////////////////////////////
Expand All @@ -17,7 +16,7 @@ interface ISuperPositions is IERC1155A {
uint256 txInfo;
address receiverAddressSP;
}

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/ISuperRBAC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { IAccessControl } from "openzeppelin-contracts/contracts/access/IAccessC
/// @dev Interface for SuperRBAC
/// @author Zeropoint Labs
interface ISuperRBAC is IAccessControl {

//////////////////////////////////////////////////////////////
// STRUCTS //
//////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/ISuperformFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/// @dev Interface for SuperformFactory
/// @author ZeroPoint Labs
interface ISuperformFactory {

//////////////////////////////////////////////////////////////
// CONSTANTS //
//////////////////////////////////////////////////////////////
Expand Down
33 changes: 16 additions & 17 deletions src/interfaces/ISuperformRouterPlusAsync.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus {
/// @notice thrown if the caller is not router plus
error NOT_ROUTER_PLUS();

/// @notice thrown if the caller is not core state registry rescuer
error NOT_CORE_STATE_REGISTRY_RESCUER();

/// @notice thrown if the rebalance to update is invalid
error COMPLETE_REBALANCE_INVALID_TX_DATA_UPDATE();

Expand All @@ -44,17 +47,17 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus {
/// @notice thrown to avoid processing the same rebalance payload twice
error REBALANCE_ALREADY_PROCESSED();

/// @notice thrown when the refund proposer is invalid
error INVALID_PROPOSER();
/// @notice thrown when the refund requester is not the payload receiver
error INVALID_REQUESTER();

/// @notice thrown when the refund payload is invalid
error INVALID_REFUND_DATA();

/// @notice thrown when refund is already proposed
error REFUND_ALREADY_PROPOSED();
/// @notice thrown when requestedrefund amount exceeds received amount
error REFUND_AMOUNT_EXCEEDS_EXPECTED_AMOUNT();

/// @notice thrown if the refund is still in dispute phase
error IN_DISPUTE_PHASE();
/// @notice thrown when the refund payload is already approved
error REFUND_ALREADY_APPROVED();

//////////////////////////////////////////////////////////////
// EVENTS //
Expand Down Expand Up @@ -168,16 +171,12 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus {
payable
returns (bool rebalanceSuccessful);

/// @notice allows the receiver / disputer to protect against malicious processors
/// @param finalPayloadId_ is the unique identifier of the refund
function disputeRefund(uint256 finalPayloadId_) external;

/// @notice allows the rescuer to propose a new refund amount after a successful dispute
/// @param finalPayloadId_ is the unique identifier of the refund
/// @param refundAmount_ is the new refund amount proposed
function proposeRefund(uint256 finalPayloadId_, uint256 refundAmount_) external;
/// @notice allows the user to request a refund for the rebalance
/// @param requestedAmount_ the amount to be refunded
function requestRefund(uint256 requestedAmount_, uint256 routerplusPayloadId_) external;

/// @notice allows the user to claim their refund post the dispute period
/// @param finalPayloadId_ is the unique identifier of the refund
function finalizeRefund(uint256 finalPayloadId_) external;
/// @dev only callable by core state registry rescuer
/// @notice approves a refund for the rebalance and sends funds to the receiver
/// @param routerplusPayloadId_ the router plus payload id
function approveRefund(uint256 routerplusPayloadId_) external;
}
Loading