076.md

J4de

medium

The lender may preemptively call updateCommitment function to modify the commitment that is about to be accepted

Summary

The lender may preemptively call updateCommitment function to modify the commitment that is about to be accepted

Vulnerability Detail

File: LenderCommitmentForwarder.sol
303     function acceptCommitment(
304         uint256 _commitmentId,
305         uint256 _principalAmount,
306         uint256 _collateralAmount,
307         uint256 _collateralTokenId,
308         address _collateralTokenAddress,
309         uint16 _interestRate,
310         uint32 _loanDuration
311     ) external returns (uint256 bidId) {
312         address borrower = _msgSender();
313
314         Commitment storage commitment = commitments[_commitmentId];
315
316         //make sure the commitment data adheres to required specifications and limits
317         validateCommitment(commitment);
318
319         //the collateral token of the commitment should be the same as the acceptor expects
320         require(
321             _collateralTokenAddress == commitment.collateralTokenAddress,
322             "Mismatching collateral token"
323         );
324         //the interest rate must be at least as high has the commitment demands. The borrower can use a higher interest rate although that would not be beneficial to the borrower.
325         require(
326             _interestRate >= commitment.minInterestRate,
327             "Invalid interest rate"
328         );
329         //the loan duration must be less than the commitment max loan duration. The lender who made the commitment expects the money to be returned before this window.
330         require(
331             _loanDuration <= commitment.maxDuration,
332             "Invalid loan max duration"
333         );
334
335         require(
336             commitmentBorrowersList[_commitmentId].length() == 0 ||
337                 commitmentBorrowersList[_commitmentId].contains(borrower),
338             "unauthorized commitment borrower"
339         );
340         //require that the borrower accepting the commitment cannot borrow more than the commitments max principal
341         if (_principalAmount > commitment.maxPrincipal) {
342             revert InsufficientCommitmentAllocation({
343                 allocated: commitment.maxPrincipal,
344                 requested: _principalAmount
345             });
346         }
347
348         uint256 requiredCollateral = getRequiredCollateral(
349             _principalAmount,
350             commitment.maxPrincipalPerCollateralAmount,
351             commitment.collateralTokenType,
352             commitment.collateralTokenAddress,
353             commitment.principalTokenAddress
354         );
355
356         if (_collateralAmount < requiredCollateral) {
357             revert InsufficientBorrowerCollateral({
358                 required: requiredCollateral,
359                 actual: _collateralAmount
360             });
361         }
362
363         //ERC721 assets must have a quantity of 1
364         if (
365             commitment.collateralTokenType == CommitmentCollateralType.ERC721 ||
366             commitment.collateralTokenType ==
367             CommitmentCollateralType.ERC721_ANY_ID
368         ) {
369             require(
370                 _collateralAmount == 1,
371                 "invalid commitment collateral amount for ERC721"
372             );
373         }
374
375         //ERC721 and ERC1155 types strictly enforce a specific token Id.  ERC721_ANY and ERC1155_ANY do not.
376         if (
377             commitment.collateralTokenType == CommitmentCollateralType.ERC721 ||
378             commitment.collateralTokenType == CommitmentCollateralType.ERC1155
379         ) {
380             require(
381                 commitment.collateralTokenId == _collateralTokenId,
382                 "invalid commitment collateral tokenId"
383             );
384         }
385
386         bidId = _submitBidFromCommitment(
387             borrower,
388             commitment.marketId,
389             commitment.principalTokenAddress,
390             _principalAmount,
391             commitment.collateralTokenAddress,
392             _collateralAmount,
393             _collateralTokenId,
394             commitment.collateralTokenType,
395             _loanDuration,
396             _interestRate
397         );
398
399         _acceptBid(bidId, commitment.lender);
400
401         _decrementCommitment(_commitmentId, _principalAmount);
402
403         emit ExercisedCommitment(
404             _commitmentId,
405             borrower,
406             _principalAmount,
407             bidId
408         );
409     }

When the borrower calls acceptCommitment function, it will verify the minimum principal, maximum collateral and other information, but does not verify whether the marketId is the same as expected. As a result, malicious borrowers can call updateCommitment function to modify marketId in advance.

Impact

marketId affects the loan actually received by the borrower, for example, different marketId means different marketplace fee.

Code Snippet

https://github.com/teller-protocol/teller-protocol-v2/blob/cb66c9e348cdf1fd6d9b0416a49d663f5b6a693c/packages/contracts/contracts/LenderCommitmentForwarder.sol#L300-L400

Tool used

Manual Review

Recommendation

It is recommended to check whether the marketId is expected in acceptCommitment function.