Zharta P2P Lending Protocol ERC20 Security Review Report

Lenders can use the replace_loan_lender() function to sell their loans for new offers, which are theoretically better for borrowers than the old ones. The function verifies that the current LTV (loan-to-value) of the loan is below the max_initial_ltv of the offer. Therefore, when the soft_liquidation_ltv is higher than the max_initial_ltv, the loan will generally still be safe under the new offer.

However, soft_liquidation_ltv is only validated when it is non-zero. A loan with a soft_liquidation_ltv of zero can still be created or used to replace another loan.

if offer.offer.soft_liquidation_ltv > 0:
    assert offer.offer.soft_liquidation_ltv > max_initial_ltv, "liquidation ltv le initial ltv"
    # required for soft liquidation: (1 + f) * iltv < 1
    assert (BPS + base.soft_liquidation_fee) * max_initial_ltv < BPS * BPS, "initial ltv too high"
 
    ...
 
if new_loan.soft_liquidation_ltv > 0:
    assert new_loan.soft_liquidation_ltv >= loan.soft_liquidation_ltv, "liquidation ltv lt old loan"

When soft_liquidate_loan() is called for a loan with a soft_liquidation_ltv of zero, the transaction always succeeds if the current LTV is higher than the loan's initial LTV (max_initial_ltv)n . Otherwise, it will revert in _compute_soft_liquidation() due to an underflow during calculation.

def soft_liquidate_loan(loan: base.Loan):
 
    """
    @notice Settle a loan.
    @param loan The loan to be settled.
    """
 
    assert base._is_loan_valid(loan), "invalid loan"
    assert not base._is_loan_defaulted(loan), "loan defaulted"
    liquidator: address = msg.sender if not base.authorized_proxies[msg.sender] else tx.origin
 
    current_interest: uint256 = base._compute_settlement_interest(loan)
    convertion_rate: base.UInt256Rational = self._get_oracle_rate()
    current_ltv: uint256 = self._compute_ltv(loan.collateral_amount, loan.amount + current_interest, convertion_rate)
 
    assert current_ltv >= loan.soft_liquidation_ltv, "ltv lt liquidation ltv"
 
    principal_written_off: uint256 = 0
    collateral_claimed: uint256 = 0
    liquidation_fee: uint256 = 0
    principal_written_off, collateral_claimed, liquidation_fee = self._compute_soft_liquidation(
        loan.collateral_amount,
        loan.amount + current_interest,
        loan.initial_ltv,
        loan.soft_liquidation_fee,
        convertion_rate,
    )
    ...

Therefore, a malicious lender can exploit this vulnerability to liquidate a healthy loan by replacing it with a new offer where soft_liquidation_ltv is zero. After that, if the loan's LTV is higher than the initial LTV, it can be soft liquidated, while it hasn't reached the original soft_liquidation_ltv. Using this attack, malicious lender can seize a large portion of a borrower's collateral after a price change.

Scenario:

1. A borrower takes a loan from a lender with an initial LTV of 70% and a soft_liquidation_ltv of 90%.
2. The lender replaces the loan with the similar offer which has a lower APR, but sets soft_liquidation_ltv to 0.
3. Now, the loan can be soft-liquidated whenever the LTV exceeds 70%.
4. When loan's LTV reaches 77% (which would still be safe under the original offer), the lender can execute a soft liquidation and claim 24% of the borrower's collateral.

Consider requiring soft_liquidation_ltv to be greater than 0, or skipping that case in the soft_liquidate_loan() function.

The lender of a signed offer can use the revoke_offer function to revoke a specific offer ID. This offer ID is the hash of the signature (v, r, s) tuple.

However, due to a missing check for signature malleability in the signature verification, it becomes possible to create a mirrored signature that results in a different order ID but valid signed offer to bypass the cancellation and still create the loan.

@pure
@internal
def _compute_signed_offer_id(offer: SignedOffer) -> bytes32:
    return keccak256(concat(
        convert(offer.signature.v, bytes32),
        convert(offer.signature.r, bytes32),
        convert(offer.signature.s, bytes32),
    ))

@external
def revoke_offer(offer: base.SignedOffer):
 
    """
    @notice Revoke an offer.
    @param offer The signed offer to be revoked.
    """
 
    assert base._check_user(offer.offer.lender), "not lender"
    assert offer.offer.expiration > block.timestamp, "offer expired"
    assert base._is_offer_signed_by_lender(offer, offer_sig_domain_separator), "offer not signed by lender"
 
    offer_id: bytes32 = base._compute_signed_offer_id(offer)
    assert not base.revoked_offers[offer_id], "offer already revoked"
 
    base._revoke_offer(offer_id, offer)

We would recommend to check for malleable signatures in the signature verification.

The tracing_id of an offer/loan is used to track the committed_liquidity of a tracing ID instance against an offer's available_liquidity.

Thanks to the tracing_id, different loans can be issued that share the same tracing ID and can therefore use the same pool of available liquidity from the lender.

However, the tracing_id is an arbitrary bytes32 and not dependent on the lender's address. An attacker can therefore create a loan with the tracing_id of another lender to fill their committed_liquidity forcefully. The attacker can take out the liquidity without decreasing it, either by settling or defaulting on the loan.

This acts as a vector for DoS by reserving liquidity of victim offers that subsequently cannot be turned into loans anymore.

@internal
def _check_and_update_offer_state(offer: SignedOffer, amount: uint256):
    offer_id: bytes32 = self._compute_signed_offer_id(offer)
    assert not self.revoked_offers[offer_id], "offer revoked"
 
    commited_liquidity: uint256 = self.commited_liquidity[offer.offer.tracing_id]
    assert commited_liquidity + amount <= offer.offer.available_liquidity, "offer fully utilized"
    self.commited_liquidity[offer.offer.tracing_id] = commited_liquidity + amount
 
    if offer.offer.borrower != empty(address):
        # offer has borrower => normal offer
        self._revoke_offer(offer_id, offer)

The tracing_id should be calculated from some inputted tracing_id (that acts as a nonce/unique identifier) and the lender's address. For example, the hash keccak256(offer.lender, tracing_id) could be taken as the actual tracing_id input for the committed_liquidities mapping.

This ensures that:

• The tracing ID can still be shared across different offers of the same lender.
• A different lender cannot access the same tracing ID.

The _compute_signed_offer_id function returns a bytes32 offer ID based off of the signature for a signed offer. However, the usage of only the tuple (v, r, s) as a key is unsafe due to signature malleability.

The ecrecover function uses an elliptic curve equation, and for a single message, there can exist two valid signatures by flipping the s value.

@pure
@internal
def _compute_signed_offer_id(offer: SignedOffer) -> bytes32:
    return keccak256(concat(
        convert(offer.signature.v, bytes32),
        convert(offer.signature.r, bytes32),
        convert(offer.signature.s, bytes32),
    ))

Since the offer_id is computed using v, r, and s, changing any of these values will generate a new offer_id.

As a result, it becomes possible to bypass the existing liquidity restrictions and signature usage validation to create a second loan from the same offer and the signature that was provided by the lender.

We would recommend to add a check for s in the signature verification:

@internal
def _is_offer_signed_by_lender(signed_offer: SignedOffer, offer_sig_domain_separator: bytes32) -> bool:
    assert signed_offer.signature.s <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, "invalid signature"
 
    return ecrecover(
        keccak256(
            concat(
                convert("\x19\x01", Bytes[2]),
                abi_encode(
                    offer_sig_domain_separator,
                    keccak256(abi_encode(OFFER_TYPE_HASH, signed_offer.offer))
                )
            )
        ),
        signed_offer.signature.v,
        signed_offer.signature.r,
        signed_offer.signature.s
    ) == signed_offer.offer.lender

The function soft_liquidate_loan contains a comment that describes the settle_loan function, instead of a comment describing the soft_liquidate_loan function itself.

@external
def soft_liquidate_loan(loan: base.Loan):
 
    """
    @notice Settle a loan.
    @param loan The loan to be settled.
    """

We recommend to fix the NatSpec comment.