`PoolTemplate:redeem()::_insurance`'s data location should be `memory`

[code423n4]

Handle

Dravee

Vulnerability details

Impact

The current number of SLOADs exceeds the amount that would occur with a copy of the struct in memory

Proof of Concept

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L549 In this particular case, the variable Insurance storage _insurance = insurances[_id]; is read more times via SLOADs in the redeem() function than what's necessary to copy the object in memory (7 SLOADs). Therefore, it's a good idea to copy it in memory via Insurance memory _insurance = insurances[_id]; and access the data via MLOADs afterwards

Tools Used

VS Code

Recommended Mitigation Steps

Change Insurance storage _insurance = insurances[_id]; to Insurance memory _insurance = insurances[_id];

[oishun1112]

https://github.com/code-423n4/2022-01-insure/blob/main/contracts/PoolTemplate.sol#L583

This line changes its state, so cannot be memory. Also, I did some experiment to use memory.

contract Storage {
    event Redeemed(
        uint256 indexed id,
        address insured,
        bytes32 target,
        uint256 amount,
        uint256 payout
    );

    struct Insurance {
        uint256 id; //each insuance has their own id
        uint256 startTime; //timestamp of starttime
        uint256 endTime; //timestamp of endtime
        uint256 amount; //insured amount
        bytes32 target; //target id in bytes32
        address insured; //the address holds the right to get insured
        bool status; //true if insurance is not expired or redeemed
    }

    mapping(uint256 => Insurance) public insurances;

    struct Incident {
        uint256 payoutNumerator;
        uint256 payoutDenominator;
        uint256 incidentTimestamp;
        bytes32 merkleRoot;
    }
    Incident public incident;

    function setInsurance()external{
        Insurance memory _insurance = Insurance(0, 0, 100, 100, "0x", 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, true);

        insurances[0] = _insurance;
    }

    function setIncident()external{
        Incident memory _incident = Incident(100, 100, 50, "0x");

        incident = _incident;
    }

    function redeem_1(uint256 _id) external {
        Insurance storage _insurance = insurances[_id];
        require(_insurance.status == true, "ERROR: INSURANCE_NOT_ACTIVE");

        uint256 _payoutNumerator = incident.payoutNumerator;
        uint256 _payoutDenominator = incident.payoutDenominator;
        uint256 _incidentTimestamp = incident.incidentTimestamp;
        bytes32 _targets = incident.merkleRoot;

        require(_insurance.insured == msg.sender, "ERROR: NOT_YOUR_INSURANCE");
        require(
                _insurance.startTime <= _incidentTimestamp &&
                _insurance.endTime >= _incidentTimestamp,
            "ERROR: INSURANCE_NOT_APPLICABLE"
        );

        _insurance.status = false;
        uint256 A;
        A = _insurance.amount;

        uint256 _payoutAmount = (_insurance.amount * _payoutNumerator) /
            _payoutDenominator;

        emit Redeemed(
            _id,
            msg.sender,
            _insurance.target,
            _insurance.amount,
            _payoutAmount
        );

        require(insurances[_id].status == false);
    }

    function redeem_2(uint256 _id) external {
        Insurance memory _insurance = insurances[_id];
        require(_insurance.status == true, "ERROR: INSURANCE_NOT_ACTIVE");

        uint256 _payoutNumerator = incident.payoutNumerator;
        uint256 _payoutDenominator = incident.payoutDenominator;
        uint256 _incidentTimestamp = incident.incidentTimestamp;
        bytes32 _targets = incident.merkleRoot;

        require(_insurance.insured == msg.sender, "ERROR: NOT_YOUR_INSURANCE");
        require(
                _insurance.startTime <= _incidentTimestamp &&
                _insurance.endTime >= _incidentTimestamp,
            "ERROR: INSURANCE_NOT_APPLICABLE"
        );

        insurances[_id].status = false;
        uint256 A;
        A = _insurance.amount;

        uint256 _payoutAmount = (_insurance.amount * _payoutNumerator) /
            _payoutDenominator;

        emit Redeemed(
            _id,
            msg.sender,
            _insurance.target,
            _insurance.amount,
            _payoutAmount
        );

        require(insurances[_id].status == false);
    }

}

redeem_1 => 47869 gas redeem_2 => 50044 gas

tried to use memory, and did insurances[_id].status = false; directly. This way didn't reduce gas

[0xean]

closing as invalid due to the assignment