Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨Linkdrop x Gnosis Safe Smart Contract Bug-bounty✨ #5

Open
Kisgus opened this issue Jul 31, 2019 · 10 comments
Open

✨Linkdrop x Gnosis Safe Smart Contract Bug-bounty✨ #5

Kisgus opened this issue Jul 31, 2019 · 10 comments

Comments

@Kisgus
Copy link
Contributor

Kisgus commented Jul 31, 2019

Scope

Below smart contracts are within the scope of the bug bounty:

Contracts within scope

LinkdropModule.sol (and all contracts it is inherited from)
https://github.com/LinkdropProtocol/linkdrop-safe-module/blob/dev/contracts/module/LinkdropModule.sol

Contracts not within scope
https://github.com/LinkdropProtocol/linkdrop-safe-module/tree/dev/contracts/imports
https://github.com/LinkdropProtocol/linkdrop-safe-module/tree/dev/contracts/mocks

Payout

Minor discovered bugs, making linkdrop behave in an unexpected harmful way, without putting any funds at risk, will be rewarded with 100 DAI.

Critical vulnerability bugs allowing 3rd parties to steal or lock up funds will be rewarded with 500 DAI.

First come first served, only the first to identify a specific minor or critical bug will be entitled to receive the payout.

Responsible Disclosure

Make sure that you do not share your submission public until we have confirmed it to you, or else you will be disqualified. Issues will be credited on a first come — first serve basis. Issues already known to us or issues already submitted by another user will not be eligible for rewards.
Issues can be submitted anonymously.

Submit an empty "submission" via Gitcoin and send your "submission" to hi@linkdrop.io

@gitcoinbot
Copy link

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


This issue now has a funding of 500.0 DAI (500.0 USD @ $1.0/DAI) attached to it as part of the LinkdropHQ fund.

@L-KH
Copy link

L-KH commented Jul 31, 2019

Costly loop

    {
        setManager();
        for(uint i = 0; i < _linkdropSigners.length; i++)
            isLinkdropSigner[_linkdropSigners[i]] = true;
    }

Ethereum is a very resource-constrained environment. Prices per computational step are orders of magnitude higher than with centralized providers. Moreover, Ethereum miners impose a limit on the total number of gas consumed in a block. If array.length is large enough, the function exceeds the block gas limit, and transactions calling it will never be confirmed
https://solidity.readthedocs.io/en/develop/security-considerations.html#gas-limit-and-loops

@gitcoinbot
Copy link

gitcoinbot commented Aug 1, 2019

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


Work has been started.

These users each claimed they can complete the work by 2 years, 6 months ago.
Please review their action plans below:

1) igor-dulger has started work.

Load code and look for vulnerabilities
2) maskerwind has started work.

1.Start by analyzing the contract with my own developed tools at first.
2.Manually check the vulnerabilities of the contract.
3.Submit the result once a bug is found.
3) robertmcforster has started work.

I'm doing mostly just manual review on the contract. Submitted a bug directly to the team and am now formally submitting it here.

Learn more on the Gitcoin Issue Details page.

@igor-dulger
Copy link

Do you have business logic description ? Formal verification won't give a lot of use, you can use some tool for this. For me to check if everything ok I need to understand what your app does.
I hope it make sense.

@amiromayer
Copy link

@L-KH Hey there, thanks for the feedback, but we don't see it as a vulnerability.
Usually this array will take one or two addresses only.
And the Gnosis Safe team itself is also using a for loop for the setup function.

@amiromayer
Copy link

@igor-dulger Sure, please refer to our technical description

@RobertMCForster
Copy link

There's a replay attack that enables malicious users to send transactions from the same signers multiple times. This vulnerability arises if the same signers have multiple linkdrop modules. Because address(this) is not included in the data to be signed, the same claim signature can be used on every linkdrop module the signers have control over, thereby sending a single approved transaction to the receiver multiple different times.

This issue has been fixed after initial reports to the team with address(this) being added in the verifyLinkdropSignerSignature functions on both the ERC20 and ERC721 modules.

@gitcoinbot
Copy link

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


Work for 500.0 DAI (500.0 USD @ $1.0/DAI) has been submitted by:

  1. @robertmcforster

@Kisgus please take a look at the submitted work:


@gitcoinbot
Copy link

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


The funding of 500.0 DAI (500.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @RobertMCForster.

@Roundtree-Larry
Copy link

LinkdropCommon.claimedTo (LinkdropCommon.sol#11) is never initialized. It is used in:
- isClaimedLink (LinkdropCommon.sol#44-49)

Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-state-variables

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants