hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Several functions has to be marked internal inside of the Router contract as it’s a base contract for other routers #37

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x57796cb5bc251f4258e3d981eda97c011cac4b00b182fec383ce6ed3b7c2f8a1 Severity: low

Description: Description\

At the moment Router contract serves as a base contract for MainnetRouter and GnosisRouter. The problem is that inside of the Router the functions splitPositions , mergePositions() and redeemPositions() are marked as public even though they are not supposed to be used.

Attack Scenario\

Currently splitPositions() inside of the Router smart contract is marked as public:

Router.sol::37

    function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public

Same thing with redeemPositions() and mergePositions():

Router.sol::90

function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public

Router.sol::143

  function redeemPositions(IERC20 collateralToken, Market market, uint256[] calldata outcomeIndexes) public {

But they are not supposed to be public as this allows users to call the functions with the collateral tokens they like and therefore bypassing the functionality from the MainnetRouter and BaseRouter.

Recommendation Make functions internal instead of public as MainnetRouter and BaseRouter inherit from the Router and the users has to be not able to enter with different collateral token other than DAI

greenlucid commented 1 month ago

But they are not supposed to be public as this allows users to call the functions with the collateral tokens they like and therefore bypassing the functionality from the MainnetRouter and BaseRouter.

They are supposed to be used, at they allow dealing with any ERC20, not just DAI / sDAI

xyzseer commented 1 month ago

they are used if you split/merge from/to sDAI

rodiontr commented 1 month ago

But they are not supposed to be public as this allows users to call the functions with the collateral tokens they like and therefore bypassing the functionality from the MainnetRouter and BaseRouter.

They are supposed to be used, at they allow dealing with any ERC20, not just DAI / sDAI

sDAI / DAI is claimed to be the only collateral here:

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/16 https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/4

rodiontr commented 1 month ago

they are used if you split/merge from/to sDAI

The functions that are responsible for this are the functions in the MainnetRouter and GnosisRouter

@xyzseer @greenlucid Can you please reconsider this?

clesaege commented 1 month ago

Having functions that are not used yet is not an issue (we could in the future extend the use to other ERC20).

As per contest rules, are excluded: