Web: Gitcoin Grants Smart Contract Bug Bounty

[captnseagraves]

Gitcoin Grants - Smart Contract Bug Bounty

Hello bounty hunters,

Gitcoin Grants is a platform to fund your open source work with recurring payments. We've developed Gitcoin Grants so that OSS Developers can crowdsource funding for the awesome work they are already doing and allow companies to provide more significant recurring funding for contributions on their open source repos.

Task

The task is to find security vulnerabilities in our Subscription smart contract. A security vulnerability is any issue that impacts the well-being of the end-user on the platform. Compensation will not be provided for visual, optmization, or interface issues.

Application details

The contract can be found at https://github.com/gitcoinco/grants1337/blob/master/Subscription/Subscription.sol. The dependencies are solidity.

Unfotunately, we do not have a truffle test suite written. We need to implement a listener in the main app in order to do so, but hope to have one up soon.

Severity OWASP model

Payouts

Payouts are structured according to the severity of the security issue raised. See aforementioned OWASP model for more info.

• Critical - 2.5ETH
• High - 1.75 ETH
• Medium - 1 ETH
• Low - .25-.50 ETH

We may receive many responses before being able to implement changes, because of this payouts will be for bugs that are unknown to us at the time of notification.

Submission

Please email any findings to [email protected].

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 2.5 ETH (521.69 USD @ $208.67/ETH) attached to it as part of the Gitcoin fund.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
  
  
  
  • Want to chip in? Add your own contribution here.
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $51,091.50 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 2.5 ETH (521.69 USD @ $208.67/ETH) attached to it as part of the Gitcoin fund.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
  
  
  
  • Want to chip in? Add your own contribution here.
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $51,091.50 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work has been started__.

These users each claimed they can complete the work by 7 months, 2 weeks from now.
Please review their action plans below:

1) liamzebedee has started work.

I will detail any vulnerability I find in detail with example code for suitable reproducibility.

Learn more on the Gitcoin Issue Details page.

[liamzebedee]

Found some stuff - but will email and not comment here? Is that what is requested? :)

[owocki]

@liamzebedee email [email protected] ?

[6ug]

Where do we need to find-out vulnerability? I mean what is in scope? I can start finding in gitcoin.co ? or only https://tokensubscription.com? @owocki

[captnseagraves]

@6ug From above: The task is to find security vulnerabilities in our Subscription smart contract. A security vulnerability is any issue that impacts the well-being of the end-user on the platform. Compensation will not be provided for visual, optmization, or interface issues.

The contract can be found at https://github.com/gitcoinco/grants1337/blob/master/Subscription/Subscription.sol. The dependencies are solidity.

[captnseagraves]

Also, please note: We may receive many responses before being able to implement changes, because of this payouts will be for bugs that are unknown to us at the time of notification.

[captnseagraves]

Response to Timur Badretdinov:

1. Timestamp dependence, especially at line 242. Can be crucial for subscriptions with a short period. I'm noting this because we can use nextValidTimestamp[subscriptionHash] instead of block.timestamp as a remedy.

• This may be an issue for subscriptions on Mainnet with periods of an hour or less. I am sure there are use cases that would want this sort term functionality, but it also becomes expensive very quickly to do... Not sure how likely it is to have a high impact. This is not an issue for Gitcoin Grants. This will be paid out via the EIP1337 ECF Grant - .5 ETH

1. A provider will lose part of payout by executing late. Again, it becomes more critical for short subscriptions, but also true for longer ones. Say, provider executed 1 day after expiration for 30-day subscription. He basically loses 3.3% of the money. Again, we can solve this by using nextValidTimestamp[subscriptionHash] on L242.

• Also, this solution gives the benefit to the provider if they forget to process a transaction. It allows them to reset the timestamp based off of the second the last period ended, instead of the second the transactions is processed. I can see why someone would want to implement this way. I would prefer giving the benefit to the subscriber. If I forget to process your transaction, you should get free service during that time. Also, when there is a network of subminers processing transactions, the economic incentive is to process the transaction as soon as possible or another subminer will do it before you.

• I suppose though, since we have isSubscriptionActive we might want to implement with nextValidTimestamp as suggested. To be paid .25 ETH from Gitcoin funds, .25 from 1337 funds

1. isSubscriptionReady lacks require() checks from executeSubscription and therefore can't fully validate tx before execution. In other words, arguments may pass isSubscriptionReady, but fail at executeSubscription.

• I ran into issues with ln235-239 during implementation on this, although I did resolve them while building. I think this is better implemented in getSubscriptionHash() than in isSubscriptionReady() though. That way you cant even create the subscription if it does not meet the requirements of the creator. Low Impact bug, more of a code refactor suggestion. To be paid from Gitcoin .125 ETH, .125 from 1337

1. Anyone can cancel any subscription of any user at any time, given signature and subscription params (from, to, token address, etc). Given that this data is public (i.e. by looking at last executeSubscription), it's pretty easy to conduct. Give the ability to cancel only to a subscriber and/or provider?

• He is right. We need to have require(from == msg.sender, 'msg.sender is not the subscriber') in cancelSubscription. Oops. Critical bug. 1.25 ETH From Gitcoin, 1.25 from 1337

1. isSubscriptionActive will show true for canceled subscription (given grace period is 0). In addition, there's no way to say that the subscription was canceled other than comparing nextValidTimestamp with uint256.MAX, which does the job, but maybe a bit non-elegant, counterintuitive, and requires knowing implementation details. Adding method isCancelled() may help.

• Right again. We could simply add a mapping of mapping(bytes32 => bool) public subHashStatus and either implement require() statements where needed or isCancelled as suggested. Medium impact bug with high likelihood. To be paid .875 from Gitcoin .875 from 1337

@owocki please pay out 2.5 ETH from Gitcoin, and 3 ETH from the 1337 ECF grant.

[captnseagraves]

Responding to @liamzebedee :

1. Exploit mechanism: proxy contracts.

Since a contract’s address is based on the deployed ABI of the contract, but not the contract’s storage, an attack vector I’ve decided to exploit is creating a contract which adheres to the ERC20 interface but whose functionality can be changed to the Subscription contract’s direct disadvantage.

The Subscription contract uses the return value of ERC20.transferFrom to ascertain whether executeSubscription was successful. The token address of Subscription cannot be changed, but the underlying contract can.

The example attached is a proxy contract using delegate call. The first contract can function like a real ERC20 token, only later to be replaced with one which always returns true from transferFrom.

Solutions: A possible fix would be to check the balance before and after, rather than relying on the value of transferFrom.

• This issue is a high impact bug with a medium likelihood. It does not apply to Grants, it is not rational for a user to not contribute anything to a Gitcoin Grant by exploiting the proxy contract vulnerability. They would simpy not begin support in the first place. It would be a waste of gas. But this does make sense in a service based subscription (Netflix like scenario). To be paid out 1.75 ETH from the 1337 ECF Grant.

Exploit mechanism: Re-entrancy attacks

If the ERC20.transferFrom call is re-entrant, meaning it will maliciously call back into the Subscription, it is possible to exploit some facts:

//increment the timestamp by the period so it wont be valid until then
nextValidTimestamp[subscriptionHash] = block.timestamp.add(periodSeconds);

This is executed before transferFrom - so a malicious actor (who specifically has engineered their own proxy contract to with a valid nonce/sig for re-entrant calls) can effectively extend their subscription period by periodSeconds every call. Infinite Netflix anyone?

• There is not actually a reentrancy bug. It does appear that there might since the timestamp is changed before the .transferFrom(), but we have a require() statement on ln228 of the bounties version that checks the validity of the timestamp on each transaction. Unless I misunderstand reentrancy bug 🤷‍♂️ . There will be no pay out for this comment.

Example code for proxy contract is here.
https://gist.github.com/liamzebedee/5c2afd3a24bb840744ab9cf149055738

@owocki please pay out 1.75 ETH from the 1337 ECF Grant to @liamzebedee he is a bounty hunter on the gitcoin bounty: https://gitcoin.co/issue/gitcoinco/web/2480/1553

[liamzebedee]

Hey @captnseagraves - not sure where the communication faltered - but on the Slack channel we discussed the re-entrancy attack. I agreed that it wasn't re-entrant, however there was still a vulnerability in terms of how signature validation did not fail when it should. Otherwise, thank you for the payout. I'll hit you up on Slack :)

[owocki]

i will pay this out as soon as i get november's ETH from consensys finance

[gitcoinbot]

⚡️ A tip worth 1.75000 ETH (374.22 USD @ $213.84/ETH) has been granted to @liamzebedee for this issue from @owocki. ⚡️

The sender had the following public comments:

Thanks for your help with the secrurity audit!

Nice work @liamzebedee! Your tip has automatically been deposited in the ETH address we have on file.

• $57123.30 in Funded OSS Work Available at: https://gitcoin.co/explorer
• Incentivize contributions to your repo: Send a Tip or Fund a PR
• No Email? Get help on the Gitcoin Slack

[gitcoinbot]

⚡️ A tip worth 3.00000 ETH (641.53 USD @ $213.84/ETH) has been granted to @Destiner for this issue from @owocki. ⚡️

The sender had the following public comments:

Thank you!

Nice work @Destiner! Your tip has automatically been deposited in the ETH address we have on file.

• $57123.30 in Funded OSS Work Available at: https://gitcoin.co/explorer
• Incentivize contributions to your repo: Send a Tip or Fund a PR
• No Email? Get help on the Gitcoin Slack

[gitcoinbot]

⚡️ A tip worth 2.50000 ETH (534.61 USD @ $213.84/ETH) has been granted to @Destiner for this issue from @owocki. ⚡️

The sender had the following public comments:

thanks again!

Nice work @Destiner! Your tip has automatically been deposited in the ETH address we have on file.

• $57123.30 in Funded OSS Work Available at: https://gitcoin.co/explorer
• Incentivize contributions to your repo: Send a Tip or Fund a PR
• No Email? Get help on the Gitcoin Slack