vulnerability about pancake-farm HOT 12 OPEN

Bumping this issue up, this migrator function is highlighted on Binance Docs itself as "malicious".
PancakeSwap is gaining traction in DeFi scene, seeing this particular function that may serve as a backdoor in extracting assets might scare away potential investors and/or adopters that are savvy enough to check the contract.

See the link below, Item 4:
https://www.binance.org/en/blog/how-to-identify-malicious-contract-on-binance-smart-chain/

Snippet

FYI
@pancake-swap @fio666 @pancake-cat

from pancake-farm.

There's an important difference between the migrate function on the Binance blog post and this one: the dangerous version of migrate gives the migrator contract infinite spending approval.

Exactly that modification is what e.g., HoneySwap used for their rug pull. This is what happened in that case:

1. Deploy the contract (this is before announcing HoneySwap to the public)
2. Set the migrator to the attacking contract
3. Call migrate for all pairs. Pools are empty so nothing is transferred but infinite approval is given as a side effect.
4. Unset the migrator (to hide the above)
5. Wait
6. The attack: just use the attack contract to transfer the funds (as it has infinite approval from step 3)

That rug pull used a previously granted spending approval which is impossible here: the migrate of PancakeSwap approves a specific balance and immediately uses that full balance, so the migrator contract cannot spend any more of PancakeSwap's LP tokens afterwards.

Also note that migrate is required for a very specific feature: the ability to migrate towards a new version of PancakeSwap.

(Note: I'm a software engineer but not seasoned in smart contracts.)

from pancake-farm.

Ok so if I think I understand this correctly, it is in FACT already removed from there smart contract code seen here:
https://bscscan.com/address/0x0e09fabb73bd3ade0a17ecc321fd13a19e81ce82#code
Can anyone confirm or correct my understanding please

from pancake-farm.

Ok so if I think I understand this correctly, it is in FACT already removed from there smart contract code seen here:
https://bscscan.com/address/0x0e09fabb73bd3ade0a17ecc321fd13a19e81ce82#code
Can anyone confirm or correct my understanding please

https://bscscan.com/address/0x73feaa1ee314f8c655e354234017be2193c9e24e#code

i think that this is the contract that you are looking for the code is in fact there i would not use pancakeswap with that backdoor there, exist alternatives to migration if fact this is worse than good for the security

from pancake-farm.

from pancake-farm.

There's an important difference between the migrate function on the Binance blog post and this one: the dangerous version of migrate gives the migrator contract infinite spending approval.

Thanks for taking the time to dive into some detail. Could you elaborate a little on where the actual difference is? The migrate function per-se looks identical.

from pancake-farm.

That's kind of scary

from pancake-farm.

Thanks! Can I ask how did you properly locate the right contract? I went to CMC and searched pancake and copied the contract address from there but apparently its wrong. How did you locate the proper one?

Appreciate the help!

from pancake-farm.

Thanks! Can I ask how did you properly locate the right contract? I went to CMC and searched pancake and copied the contract address from there but apparently its wrong. How did you locate the proper one?

Appreciate the help!

because its two diferent contracts the one you pick is the cake token contract the one i copy is the main staking contract that has the vulnerability

from pancake-farm.

Thanks @josemtm for your reply! How do you find that contract ID in first place?

from pancake-farm.

@cyberena

It is stated on the README.md (MasterChef Contract)

from pancake-farm.

Has there been any progress on this issue? Do PancakeSwap still insist it is an essential function?

from pancake-farm.