Caviar is a fully on-chain NFT AMM that allows you to trade every NFT in a collection (from floors to superrares). You can also trade fractional amounts of each NFT too. It's designed with a heavy emphasis on composability, flexibility and usability.
Scope
| File | SLOC | Description and coverage | Libraries |
|---|---|---|---|
| Contracts (3) | |||
| Caviar.sol | 26 | Factory contract that creates pairs and maintains a registry (100%) | solmate |
| Pair.sol | 212 | Pair contract that contains ERC20 AMM, NFT wrapping and NFT AMM logic (100%) | solmate openzeppelin |
| LpToken.sol | 15 | ERC20 token which represents liquidity ownership in pair contracts (100%) | solmate |
| Libraries (1) | |||
| SafeERC20Namer.sol | 65 | Helper contract that fetches the name and symbol of an ERC20/ERC721 (0%) | openzeppelin |
| Total | 318 |
내가 생각한 결함
1. baseTokenShare와 fractionalTokenShare 값 체크의 부재로 lpToken은 민팅되지 않을 수 있다.
//Pair.sol
function add(uint256 baseTokenAmount, uint256 fractionalTokenAmount, uint256 minLpTokenAmount)
public
payable
returns (uint256 lpTokenAmount)
{
// *** Checks *** //
// check the token amount inputs are not zero
require(baseTokenAmount > 0 && fractionalTokenAmount > 0, "Input token amount is zero");
// check that correct eth input was sent - if the baseToken equals address(0) then native ETH is used
require(baseToken == address(0) ? msg.value == baseTokenAmount : msg.value == 0, "Invalid ether input");
// calculate the lp token shares to mint
lpTokenAmount = addQuote(baseTokenAmount, fractionalTokenAmount);
// check that the amount of lp tokens outputted is greater than the min amount
require(lpTokenAmount >= minLpTokenAmount, "Slippage: lp token amount out");
// *** Effects *** //
// transfer fractional tokens in
_transferFrom(msg.sender, address(this), fractionalTokenAmount);
// *** Interactions *** //
// mint lp tokens to sender
lpToken.mint(msg.sender, lpTokenAmount);
// transfer base tokens in if the base token is not ETH
if (baseToken != address(0)) {
// transfer base tokens in
ERC20(baseToken).safeTransferFrom(msg.sender, address(this), baseTokenAmount);
}
emit Add(baseTokenAmount, fractionalTokenAmount, lpTokenAmount);
}
function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
uint256 lpTokenSupply = lpToken.totalSupply();
if (lpTokenSupply > 0) {
// calculate amount of lp tokens as a fraction of existing reserves
uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
return Math.min(baseTokenShare, fractionalTokenShare);
} else {
// if there is no liquidity then init
return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
}
}pair에 유동성을 공급하는 add 함수에 minLpTokenAmount가 파라미터로 들어간다. minLpTokenAmount의 최솟값 체크가 없기 때문에
lpTokenAmount의 값을 확인하는 아래 require문이 제대로 동작한다고 볼 수 없다.
require(lpTokenAmount >= minLpTokenAmount, "Slippage: lp token amount out");또한 addQuote는 lpTokenSupply가 0 보다 클 경우, baseTokenShare과 fractionalTokenShare 중 최솟값을 리턴한다. 하지만 입력받은 baseTokenAmount 값이 baseTokenReserves 대비 매우 작거나 fractionalTokenAmount 값이 fractionalTokenReserves 대비 매우 작다면 addQuote는 0을 리턴할 것이다.
_transferFrom(msg.sender, address(this), fractionalTokenAmount);
lpToken.mint(msg.sender, lpTokenAmount);따라서 유저는 fractionalTokenAmount 만큼 컨트랙트에 토큰을 전송하지만 lpToken은 민팅되지 않는다. 다음과 같이 수정하는 것이 좋을 것 같다.
function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
uint256 lpTokenSupply = lpToken.totalSupply();
if (lpTokenSupply > 0) {
// calculate amount of lp tokens as a fraction of existing reserves
uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
uint256 minValue = Math.min(baseTokenShare, fractionalTokenShare); //++++++++++++++++++++++++++++++
require(minValue > 0, "Minimum value is zero"); //+++++++++++++++++++++++++++++++++++++++++++++++++
return minValue; //+++++++++++++++++++++++++++++++++++++++++++++++++
} else {
// if there is no liquidity then init
return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
}
}2. baseToken이 이더가 아닌 경우에 구매 자금 없이 pair의 토큰이 탈취당할 수 있다.
//Pair.sol
function buy(uint256 outputAmount, uint256 maxInputAmount) public payable returns (uint256 inputAmount) {
// *** Checks *** //
// check that correct eth input was sent - if the baseToken equals address(0) then native ETH is used
require(baseToken == address(0) ? msg.value == maxInputAmount : msg.value == 0, "Invalid ether input");
// calculate required input amount using xyk invariant
inputAmount = buyQuote(outputAmount);
// check that the required amount of base tokens is less than the max amount
require(inputAmount <= maxInputAmount, "Slippage: amount in");
// *** Effects *** //
// transfer fractional tokens to sender
_transferFrom(address(this), msg.sender, outputAmount);
// *** Interactions *** //
if (baseToken == address(0)) {
// refund surplus eth
uint256 refundAmount = maxInputAmount - inputAmount;
if (refundAmount > 0) msg.sender.safeTransferETH(refundAmount);
} else {
// transfer base tokens in
ERC20(baseToken).safeTransferFrom(msg.sender, address(this), inputAmount);
}
emit Buy(inputAmount, outputAmount);
} function buyQuote(uint256 outputAmount) public view returns (uint256) {
return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
}inputAmount가 0보다 크다는 보장이 없다. 다음과 같은 상황에서 inputAmount가 0으로 되면서 문제가 발생한다.
baseTokenReserves()의 값이fractionalTokenReserves()의 값보다 매우 작고baseToken이 이더가 아닐 경우
outputAmount만큼의 토큰이 페어에서 빠져나가지만 inputAmount이 0이기 때문에 페어로 토큰이 들어오지 않는다. 즉, 구매 자금 없이 페어의 토큰을 탈취할 수 있다는 것이다. 따라서 다음과 같이 코드를 수정해야 한다.
function buyQuote(uint256 outputAmount) public view returns (uint256 inputAmount) {
uint256 inputAmount = (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
require(inputAmount > 0, "InputAmount is zero");
return inputAmount;
}새로 배운 것
//Pair.sol
function _transferFrom(address from, address to, uint256 amount) internal returns (bool) {
balanceOf[from] -= amount;
// Cannot overflow because the sum of all user
// balances can't exceed the max uint256 value.
unchecked {
balanceOf[to] += amount;
}
emit Transfer(from, to, amount);
return true;
}
_transferFrom 함수를 보고 눈을 의심했었다. _transferFrom은 토큰을 구매하거나 판매할 때 사용자의 자금을 받는 기능을 한다. 그런데 사용자가 자금을 보내는 것과 상관없이 항상 true를 리턴하기 때문에 명백한 결함이라고 생각했었다. 혹시나 싶어서 solidity by example에서 erc20 토큰 예시를 찾아봤다.
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
import "./IERC20.sol";
contract ERC20 is IERC20 {
uint public totalSupply;
mapping(address => uint) public balanceOf;
mapping(address => mapping(address => uint)) public allowance;
string public name = "Solidity by Example";
string public symbol = "SOLBYEX";
uint8 public decimals = 18;
function transfer(address recipient, uint amount) external returns (bool) {
balanceOf[msg.sender] -= amount;
balanceOf[recipient] += amount;
emit Transfer(msg.sender, recipient, amount);
return true;
}
function approve(address spender, uint amount) external returns (bool) {
allowance[msg.sender][spender] = amount;
emit Approval(msg.sender, spender, amount);
return true;
}
function transferFrom(
address sender,
address recipient,
uint amount
) external returns (bool) {
allowance[sender][msg.sender] -= amount;
balanceOf[sender] -= amount;
balanceOf[recipient] += amount;
emit Transfer(sender, recipient, amount);
return true;
}
function mint(uint amount) external {
balanceOf[msg.sender] += amount;
totalSupply += amount;
emit Transfer(address(0), msg.sender, amount);
}
function burn(uint amount) external {
balanceOf[msg.sender] -= amount;
totalSupply -= amount;
emit Transfer(msg.sender, address(0), amount);
}
}알고 보니 transferFrom이 실제로 동작하는 원리는 balanceOf를 더해주거나 빼주는 것이었다.
후기
사실 생각했던 결함이 하나 더 있다. Reentrancy 공격이 가능할 것 같아서 foundry로 테스트를 해봤는데 결국 성공하지 못 했다. 컨테스트 시간이 거의 끝나가서 해당 건은 제출하지 못 했다. 내가 테스트 코드를 제대로 안 짰거나 애초에 Reentrancy 공격이 가능한 코드가 아니었을 수 있다. 컨테스트가 끝나면 ethereum stackexchange에 질문을 올려서 어떤 점을 보완해야 했는지 알아봐야겠다.
Just BUIDL :)