Debt DAO contest - eierina's results

A cryptonative credit marketplace for fully anon and trustless loans to DAOs.

General Information

Platform: Code4rena

Start Date: 03/11/2022

Pot Size: $115,500 USDC

Total HM: 17

Participants: 120

Period: 7 days

Judge: LSDan

Total Solo HM: 1

Id: 174

League: ETH

Debt DAO

Findings Distribution

Researcher Performance

Rank: 36/120

Findings: 2

Award: $150.02

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

8.0811 USDC - $8.08

Labels

bug
2 (Med Risk)
satisfactory
duplicate-39

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L223-L244 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L265-L285 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L292-L311 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L315-L333 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L388-L409

Vulnerability details

Impact All payable functions addCredit(), increaseCredit(), depositAndClose(), depositAndRepay(), close(). in LineOfCredit do not refund if:

  • the caller sends Ethers but token is not set to Denominators.ETH
  • the caller sends Ethers in excess (msg.value > amount)

Moreover the LineOfCredit contract does not have a "post-mortem" way to withdraw and return excess Ethers from the contract as the only 3 functions that can transfer Ethers outside of the contract are borrow(), withdraw(), and _close() and all 3 follows the internal accounting, causing any excess Ethers to be locked in.

Proof of Concept

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L223-L244

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L265-L285

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L292-L311

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L315-L333

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L388-L409

Tools Used

n/a

Add some error handling to addCredit(), increaseCredit, depositAndClose(), depositAndRepay(), close(). in LineOfCredit so that any excess or unexpected Ethere amount transferred in is returned to the sender within the same transaction.

For all LineLib's xxxTokenOrETH require msg.value to be zero if token is not Denominations.ETH as the same name implies Token "OR" ETH.

For most of the functions that discussed above a change that includes the following would mitigate (requires Checks-Effects-Interactions pattern):

if(token == Denominators.ETH && msg.value > amount) { (bool success, ) = payable(msg.sender).call{value:(msg.value - amount)}(""); require(success, "Address: unable to send value, recipient may have reverted"); }

Only exception is with the close() function where at the very end of the function (requires Checks-Effects-Interactions pattern), the following can be added:

if(token == Denominators.ETH && msg.value > facilityFee) { (bool success, ) = payable(msg.sender).call{value:(msg.value - facilityFee)}(""); require(success, "Address: unable to send value, recipient may have reverted"); }

#0 - Simon-Busch

2022-12-06T15:25:03Z

This issue was created as requested by @dmvt as it's a duplicate of 25 and 89

#1 - c4-judge

2022-12-06T15:26:43Z

dmvt marked the issue as duplicate of #25

#2 - c4-judge

2022-12-06T15:26:50Z

dmvt marked the issue as full credit

#3 - C4-Staff

2022-12-20T06:44:54Z

liveactionllama marked the issue as duplicate of #39

Findings Information

Labels

bug
2 (Med Risk)
satisfactory
duplicate-355

Awards

141.941 USDC - $141.94

External Links

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L223-L244 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L265-L285 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L292-L311 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L315-L333 https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L388-L409

Vulnerability details

Impact All payable functions addCredit(), increaseCredit(), depositAndClose(), depositAndRepay(), close(). in LineOfCredit do not refund if:

  • the caller sends Ethers but token is not set to Denominators.ETH
  • the caller sends Ethers in excess (msg.value > amount)

Moreover the LineOfCredit contract does not have a "post-mortem" way to withdraw and return excess Ethers from the contract as the only 3 functions that can transfer Ethers outside of the contract are borrow(), withdraw(), and _close() and all 3 follows the internal accounting, causing any excess Ethers to be locked in.

Proof of Concept

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L223-L244

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L265-L285

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L292-L311

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L315-L333

https://github.com/debtdao/Line-of-Credit/blob/6987988fe39901cad9a8e5ebb2c6aa719590873d/contracts/modules/credit/LineOfCredit.sol#L388-L409

Tools Used

n/a

Add some error handling to addCredit(), increaseCredit, depositAndClose(), depositAndRepay(), close(). in LineOfCredit so that any excess or unexpected Ethere amount transferred in is returned to the sender within the same transaction.

For all LineLib's xxxTokenOrETH require msg.value to be zero if token is not Denominations.ETH as the same name implies Token "OR" ETH.

For most of the functions that discussed above a change that includes the following would mitigate (requires Checks-Effects-Interactions pattern):

if(token == Denominators.ETH && msg.value > amount) { (bool success, ) = payable(msg.sender).call{value:(msg.value - amount)}(""); require(success, "Address: unable to send value, recipient may have reverted"); }

Only exception is with the close() function where at the very end of the function (requires Checks-Effects-Interactions pattern), the following can be added:

if(token == Denominators.ETH && msg.value > facilityFee) { (bool success, ) = payable(msg.sender).call{value:(msg.value - facilityFee)}(""); require(success, "Address: unable to send value, recipient may have reverted"); }

#0 - c4-judge

2022-11-17T15:47:12Z

dmvt marked the issue as duplicate of #25

#1 - dmvt

2022-12-06T15:16:38Z

Note, this should be duplicated by the C4 team with the new one being a dup of #89

#2 - c4-judge

2022-12-06T15:16:51Z

dmvt marked the issue as satisfactory

#3 - C4-Staff

2022-12-20T06:05:46Z

liveactionllama marked the issue as duplicate of #355

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter