code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

inheritance (virtual function) #200

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/escrow/NFTEscrow.sol#L61

Vulnerability details

Impact

The function _encodeFlashEscrowPayload() can be overriden by another function.

Proof of Concept

Solidity 201: '102. Solidity supports multiple inheritance including polymorphism:

  1. Function Overriding: Base functions can be overridden by inheriting contracts to change their behavior if they are marked as virtual. The overriding function must then use the override keyword in the function header.'

I don't understand whythis function should be overridden? This is highly risky. Also there is no overriding function in the contract (override keyword)

Tools Used

VSC

Recommended Mitigation Steps

Remove the inheritance for this function (virtual keyword)

spaghettieth commented 2 years ago

The function is overridden in both CryptoPunksHelper and EtherRocksHelper. Also from your issue, I'd suggest looking up how inheritance works in detail as I think your ideas are a bit confused.

dmvt commented 2 years ago

Invalid