Company Simulator
First Flight #51
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

Smart contract wallets without a default payable function will not be able to withdraw shares

darkfox

Description

When investors call withdraw_shares to redeem shares for ETH, Cyfrin_Hub attempts a raw_call to send the ETH to the caller. However, if the shareholder is a smart contract without a default payable function, the call will fail and the withdraw will revert. Therefore, shares held by one of these contracts will be unredeemable.

@external
def withdraw_shares():
​
...
​
@> raw_call(
msg.sender,
b"",
value=payout,
revert_on_failure=True,
)
​
...

Risk

Likelihood:

The shareholder must be a smart contract wallet without the ability to receive payment.

Impact:

The affected shareholder will never be able to redeem their shares, and the shares will be perminently out of circulation.

Proof of Concept

Create this DeadShares contract in tests/unit/attackContracts.

company: immutable(address)
​
@deploy
def __init__(_company: address):
company = _company
@payable
@external
def buy_shares():
data: Bytes[36] = concat(
method_id("fund_cyfrin(uint256)"), convert(1, bytes32)
)
raw_call(company, data, value=msg.value, revert_on_failure=True)
​
@external
def sell_shares():
data: Bytes[4] = method_id("withdraw_shares()")
raw_call(
company,
data,
revert_on_failure=True,
)

Then add this test in tests/unit/test_Industry.py.

def test_BadContractCantSellShares(industry_contract, OWNER, PATRICK):
boa.env.set_balance(OWNER, SET_OWNER_BALANCE)
with boa.env.prank(OWNER):
industry_contract.fund_cyfrin(0, value=SET_OWNER_BALANCE)
with boa.env.prank(PATRICK):
dead_shares_contract = boa.load("tests/unit/attackContracts/DeadShares.vy", industry_contract.address)
dead_shares_contract.buy_shares(value=boa.env.get_balance(PATRICK))
with boa.env.prank(dead_shares_contract.address):
print(f"Attacker shares: {industry_contract.get_my_shares()}")
​
with boa.reverts():
with boa.env.prank(PATRICK):
dead_shares_contract.sell_shares()

This test will show that attempts for the contract to withdraw ETH for the shares will always revert.

Recommended Mitigation

You should not be pushing ETH inside withdraw_shares. Instead, consider adopting a method for users to pull ETH from the contract rather than push it to them. This could be done but setting pull amounts in a mapping during withdraw_shares and creating another function for a user to pull out the funds. Even if a smart contract wallet doesn't redeem their ETH, the shares will then be available for other investors.

You could also consider adding a function to convert the ETH into WETH and send it to the contract if they are unable to redeem the ETH directly. This could be callable by any EOA on behalf of the smart contract wallet.

Updates

Lead Judging Commences

0xshaedyw Lead Judge
9 days ago
0xshaedyw Lead Judge 7 days ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.