Skip to content

Latest commit

 

History

History
77 lines (48 loc) · 3.21 KB

046.md

File metadata and controls

77 lines (48 loc) · 3.21 KB

Cold Topaz Cottonmouth

Medium

[M-1] Lack of Error Handling in PredictDotLoan::_getFulfillment function leads to unexpected behaviors

Vulnerability Description:

In the PredictDotLoan::_getFulfillment function, there is a missing validation to make sure that the Fulfillment struct is initialized, all the time before retrieving values . Although we are signing the Proposal Off-Chain but still It doesn't entirely remove the need for on-chain error handling.

https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L952

function _getFulfillment(Proposal calldata proposal) private view
    returns (Fulfillment storage fulfillment) {
        fulfillment = fulfillments[keccak256
            (abi.encodePacked(proposal.from, proposal.salt, proposal.proposalType))];
@>    // Missing Validation
}

The Fulfillment struct is retrieved using the key. However, if no entry exists in the fulfillments mapping for this key, code will return an uninitialized struct.

This is because when we access a non-existing key in a mapping, it does not revert or throw an error. It just returns the default values.

In this case, the default Fulfillment struct might have:

proposalId: bytes32(0),
collateralAmount: 0,
loanAmount: 0.

Even though the fulfillment is processed off-chain, when you submit or reference it on-chain. There is still possibility that incorrect data being passed, either due to user error or manipulation.

Although off-chain processing may reduce the risk of bad data being created, it does not eliminate the need for on-chain validation. Therefore, error handling is still necessary to protect against potential edge cases, invalid data, or exploits.

Impact:

This can lead to silent failures in protocol. The silent return of an uninitialized Fulfillment is dangerous.

If code assumes that a fulfillment exists but it operates on a default struct that will result in incorrect logic (like lending 0 amount). It's a simple check but can lead to disturb protocol business logic by just not having a validation check

Internal Pre-Conditions: Failure or distrubtion in Off-Chain signing of Proposals

Proof of Concept:

An attacker could try to manipulate the off-chain fulfillment process and then submit invalid or tempered data to the contract. Thus, the contract does not have any validation on data retrieved from PredictDotLoan::fulfillments mapping , it will result in processing malicious actions.

For Example:

 if someone submits a Fulfillment with loanAmount = 0,
 the contract might proceed as if the loan is valid, 
leading to undesirable outcomes and compromising the integrity of protocol.

Recommended Mitigation: Even with off-chain processing, protocol should still include on-chain validation checks when accessing or using the Fulfillment.

Specifically: Ensure that the Fulfillment exists in the fulfillments mapping.

- 
+   require(fulfillments[**fulfillment_key**].loanAmount > 0, "Fulfillment not found or invalid");

Even though the Fulfillment is processed off-chain, on-chain validation ensures that the data is valid and present before the contract operates on it.