Updating Wallet Addresses Security

[0x4007]

One of my colleagues had a security related concern about updating wallet addresses. Can you walk me through how an exploit could occur, and what measures are being put into place to combat these vectors?

The scenario he presented was if somebody is able to update all the wallets in our system and then front-run/claim new bounties posted. I think its unlikely to occur but I didn't have an answer for him as I am not 100% clear on the full architecture related to this system.

Originally posted by @pavlovcik in #138 (comment)

[0xcodercrane]

I think your colleagues caught up on a good point regarding security. If we have automatic payment live without considering security, there may be an attack vector. because every step is fully automatic starting from /assign to the process of payment.

I am still not sure how the attack vector looks like but we SHOULD dig into it and make the automatic payment live after that.

[0xcodercrane]

I will keep posting my research about that

[rndquu]

One of my colleagues had a security related concern about updating wallet addresses. Can you walk me through how an exploit could occur, and what measures are being put into place to combat these vectors?

The scenario he presented was if somebody is able to update all the wallets in our system and then front-run/claim new bounties posted. I think its unlikely to occur but I didn't have an answer for him as I am not 100% clear on the full architecture related to this system.

Originally posted by @pavlovcik in #138 (comment)

Yes it is possible if supabase is compromised.

An attacker can update all wallet addresses and bounty hunters, if not paying enough attention to a withdraw address, can claim future bounties directly to the attacker's address.

All currently unclaimed bounties won't be affected because attacker would need a bot's private key to modify permits. But if the attacker has the bot's private key it is simpler for the attacker to move funds directly from the bot's address.

For now I would restrict automatic payments only for amounts, say less than 100$. Then, when we intergrate multisig, we could remove this restriction.

Another layer of security is simply adding a banner "check the withdraw address" at https://pay.ubq.fi/

[rndquu]

We can invalidate a permit nonce https://github.com/Uniswap/permit2/blob/main/src/SignatureTransfer.sol#L130

[0xcodercrane]

Possible attack vectors

1/ Update wallet addresses for hunters by compromising supabase project.
This case would happen in case supabase key and project id is compromised. Exploiters can update the wallet table whenever they want.

2/ Drain funds from the admin wallet by acting as an assignee by exploiting the normal bounty process.
Here's a brief workflow of how the current system works.
create a bounty --> discussions --> assign --> working on PR --> request for review --> merge PR --> payout

[x] Anyone can be assigned by himself using /assign command without any admin's confirmation. he can be the assignee of the bounty before payment is made --- should check its possibility
[x] Is it possible for hunters to update either one in time/priority/price labels of the bounty? If possible, who can and how?
[x] We've been focusing on the stability of the bounty bot product and really spent most of our time on it but to be honest, I don't think we have all the possible validation/checks to prevent attack vectors.

How to prevent

It should be continuously tried to figure out. It looks like either of us doesn't have a clear solution to prevent it. my quick suggestion is to add admins confirmation to both assign feature and permit2 feature at least. would love to hear your comments.

[0x4007]

1

We could look into IP/geolocation whitelisting.

2

The PR needs to be accepted by a trusted team member (a "bounty master")

3

We should only allow /assign if the assignee is currently the open devpool.

4

Only bounty masters can update labels. More technically speaking, people who have been directly invited to contribute on the repo and added as a member of the organization.

5

That's okay, we learn in production and iterate.

---

• I think admin confirmation is not necessary if we only enable /assign when the assignee is the DevPool. It could be interesting if they are added to a queue, where if the first hunter gets eliminated, the bot automatically assigns the next person who wrote /assign

• /permit2 feature referring to payout. This technically already has an admin confirmation as the admins are the ones that mark the issue as closed, which should be the condition that tells the bot to generate and comment the permit.

[0xcodercrane]

It might be problematic if hunters can close the issue/bounty themselves.

[0x4007]

Bounty hunters can not close issues themselves.

Technically the closest they can do is if we have the repo setting for auto close issue with merged/linked PR...but the admin has to approve and merge it first.