search

code-423n4 / 2021-04-basedloans-findings

0 stars 1 forks source link

Reliance on the fact that NO_ERROR = 0 #2

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

gpersoon

Vulnerability details

Impact

In several occasions it's relied upon that the error value NO_ERROR is equivalent to 0. I haven't seen any real problems with this. However in most locations there is an explicit check for NO_ERROR and comparing with 0 allows for possible future mistakes (especially if the enums would change).

Proof of Concept

CToken.sol:
function transferTokens( ... if (allowed != 0) {

function mintFresh( ... if (allowed != 0) {

function redeemFresh( ... if (allowed != 0) {

function borrowFresh( ... if (allowed != 0) {

function repayBorrowFresh( ... if (allowed != 0) {

function liquidateBorrowFresh( ... if (allowed != 0) {

function seizeInternal( ... if (allowed != 0) {

Comptroller.sol: function exitMarket( ... if (allowed != 0) { ... require(oErr == 0, "exitMarket: getAccountSnapshot failed"); // semi-opaque error code

function getHypotheticalAccountLiquidityInternal( ... if (oErr != 0) { // semi-opaque error code, we assume NO_ERROR == 0 is invariant between upgrades

Tools Used

Grep

Recommended Mitigation Steps

Replace 0 with the appropriate version of ...NO_ERROR

ghoul-sol commented 3 years ago

Added to backlog.