code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

Certain view functions should never be used in code, only UI. They are easily manipulated. #154

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

tensors

Vulnerability details

Impact

The view functions in StablesConverter.sol can be manipulated to give incorrect answers by flashloan attacks. Using them within the code in a naive way can lead to lost funds.

Example

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/converters/StablesConverter.sol#L169

Recommendations

Make sure the functions are only used as estimates (for example, on the UI). Furthermore, add a comment so that new developers working on the code base realize the same thing. Dont use this function in the rest of the code.

Haz077 commented 2 years ago

It is not used in anywhere else in the code so I don't think this is an issue.

uN2RVw5q commented 2 years ago

It is used in LegacyController https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/LegacyController.sol#L164

As far as I understand, LegacyController.withdraw can only be called by the legacy metavault contract. If this is a trusted / whitelisted user, then I would say that there is no risk. Would be useful if @Haz077 or another dev could confirm this. If this is the case, then changing the tag to "sponsor disputed" sounds good to me.

transferAndCall commented 2 years ago

Yes, the Metavault is trusted as it doesn't allow contract depositors/withdrawers.

GalloDaSballo commented 2 years ago

Agree with sponsor

Note for sponsor: There's an EIP that may not allow you to disable contracts from interacting, see: https://eips.ethereum.org/EIPS/eip-3074