002.md

Proper Daffodil Toad

High

Inconsistent attestation expiration in attestLender and attestBorrower functions result in unauthorized actions or access in the market

Summary

The contract suffers from an inconsistent attestation expiration issue, leading to the possibility of stakeholders (lenders and borrowers) being incorrectly marked as "attested" beyond their actual expiration time. This could result in unauthorized actions or access in the market, including attestation and participation in a market after expiration.

Root Cause

The vulnerability lies within the attestLender and attestBorrower functions. When a lender or borrower is attested, they are granted certain permissions within the market, such as the ability to participate. However, the expiration time is not enforced consistently, which allows these stakeholders to continue having the permissions even after their attestation has expired. https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/MarketRegistry.sol#L289-L295 https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/MarketRegistry.sol#L367-L373

function attestLender(
    uint256 _marketId,
    address _lenderAddress,
    uint256 _expirationTime
) external {
    _attestStakeholder(_marketId, _lenderAddress, _expirationTime, true);
}

function attestBorrower(
    uint256 _marketId,
    address _borrowerAddress,
    uint256 _expirationTime
) external {
    _attestStakeholder(_marketId, _borrowerAddress, _expirationTime, false);
}

function _attestStakeholder(
    uint256 _marketId,
    address _stakeholderAddress,
    uint256 _expirationTime,
    bool _isLender
) internal {
    // Attestation logic
    if (_expirationTime > block.timestamp) {
        // Attestation is valid
        if (_isLender) {
            // Attest lender
            markets[_marketId].verifiedLendersForMarket.add(_stakeholderAddress);
        } else {
            // Attest borrower
            markets[_marketId].verifiedBorrowersForMarket.add(_stakeholderAddress);
        }
    }
    // There should be an expiration check, but it's not present
}

In the above code, while the expiration time is passed as a parameter, there is no proper expiration check against the current time (block.timestamp). This means that even if the expiration time has passed, stakeholders remain attested without being removed from the market.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Attest a lender or borrower with an expiration time.
2. Try to perform actions related to the attestation after the expiration time has passed.
3. Verify that the stakeholder can still perform actions after their attestation has expired.

Impact

This vulnerability allows attested stakeholders (lenders and borrowers) to continue acting in the market after their attestation has expired. The implications include:

1. Expired lenders or borrowers could still participate in a market and perform actions like making bids or entering agreements.
2. A lender who is no longer authorized might still provide funding, or a borrower could enter a loan agreement after the expiration of their attestation.
3. Since market behaviors and transactions may depend on validated actors (lenders and borrowers), expired attestations could disrupt the integrity of the market or lead to unauthorized access.

PoC

pragma solidity ^0.8.0;

import "ds-test/test.sol";  // Use for testing
import { MarketRegistry } from "./MarketRegistry.sol";  // The contract we are testing

contract TestAttestationExpiration is DSTest {
    MarketRegistry marketRegistry;

    uint256 marketId;
    address lender = address(0x123);
    address borrower = address(0x456);
    uint256 expirationTime;

    function setUp() public {
        marketRegistry = new MarketRegistry();
        marketId = 1; // Example market ID
        expirationTime = block.timestamp + 1 hours;  // Set expiration time to 1 hour from now
    }

    function testAttestationExpiration() public {
        // Attest a lender with expiration time
        marketRegistry.attestLender(marketId, lender, expirationTime);
        // Wait for 2 hours to simulate expiration
        vm.warp(block.timestamp + 2 hours);
        
        // Attempt to perform an action after expiration
        bool canLenderExit = marketRegistry.lenderExitMarket(marketId); 
        
        // Expectation: The lender should not be able to exit the market after attestation expires
        assertEq(canLenderExit, false, "Lender should not be able to exit after expiration");
    }
}

The test is expected to fail because the lender is able to exit the market despite the expiration time having passed. This shows that the expiration check is missing or improperly handled.

Fail: Lender should not be able to exit after expiration

Mitigation

To fix the issue, the code should enforce proper expiration checks to remove lenders or borrowers after their attestation expires. The _attestStakeholder function should be modified to reject actions when the attestation has expired.

function _attestStakeholder(
    uint256 _marketId,
    address _stakeholderAddress,
    uint256 _expirationTime,
    bool _isLender
) internal {
    // Ensure expiration time is greater than the current block timestamp
    require(_expirationTime > block.timestamp, "Attestation has expired");

    // Proceed with attestation as usual
    if (_isLender) {
        markets[_marketId].verifiedLendersForMarket.add(_stakeholderAddress);
    } else {
        markets[_marketId].verifiedBorrowersForMarket.add(_stakeholderAddress);
    }
}