457 lines
12 KiB
Markdown
457 lines
12 KiB
Markdown
|
|
# 🔒 Slither 安全审计修复指南
|
||
|
|
|
||
|
|
## 📌 修复优先级
|
||
|
|
|
||
|
|
- **P0 (立即修复)**: 高危漏洞,可能导致资金损失或合约被破坏
|
||
|
|
- **P1 (尽快修复)**: 中危问题,可能导致精度损失或逻辑错误
|
||
|
|
- **P2 (建议修复)**: 低危问题,影响代码质量和可维护性
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🔴 P0: 高危问题 (必须立即修复)
|
||
|
|
|
||
|
|
### 1. ⚠️ 未保护的初始化函数 (最严重!)
|
||
|
|
|
||
|
|
**问题**: 所有可升级合约的 `initialize` 函数缺少 `initializer` 修饰符
|
||
|
|
|
||
|
|
**风险**: 攻击者可以调用 `upgradeToAndCall` 删除合约!
|
||
|
|
|
||
|
|
**受影响的合约**:
|
||
|
|
- YTAssetFactory.sol
|
||
|
|
- YTAssetVault.sol
|
||
|
|
- YTPoolManager.sol
|
||
|
|
- YTVault.sol
|
||
|
|
- YTPriceFeed.sol
|
||
|
|
- YTRewardRouter.sol
|
||
|
|
- USDY.sol
|
||
|
|
- WUSD.sol
|
||
|
|
- YTLPToken.sol
|
||
|
|
|
||
|
|
#### ✅ 修复方案
|
||
|
|
|
||
|
|
**YTAssetFactory.sol (Line 52-63)**
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前
|
||
|
|
function initialize(
|
||
|
|
address _vaultImplementation,
|
||
|
|
uint256 _defaultHardCap
|
||
|
|
) external initializer {
|
||
|
|
if (_vaultImplementation == address(0)) revert InvalidAddress();
|
||
|
|
|
||
|
|
__Ownable_init(msg.sender);
|
||
|
|
__UUPSUpgradeable_init();
|
||
|
|
|
||
|
|
vaultImplementation = _vaultImplementation;
|
||
|
|
defaultHardCap = _defaultHardCap;
|
||
|
|
}
|
||
|
|
|
||
|
|
// ✅ 修复后 (已经有 initializer 修饰符,这个合约是正确的)
|
||
|
|
// 但需要确保其他合约也添加了
|
||
|
|
```
|
||
|
|
|
||
|
|
**YTAssetVault.sol (Line 125-154) - 需要修复**
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前
|
||
|
|
function initialize(
|
||
|
|
string memory _name,
|
||
|
|
string memory _symbol,
|
||
|
|
address _manager,
|
||
|
|
uint256 _hardCap,
|
||
|
|
address _wusd,
|
||
|
|
uint256 _redemptionTime,
|
||
|
|
uint256 _initialWusdPrice,
|
||
|
|
uint256 _initialYtPrice
|
||
|
|
) external initializer {
|
||
|
|
// ... 初始化代码
|
||
|
|
}
|
||
|
|
|
||
|
|
// ✅ 修复后 (已正确)
|
||
|
|
// 这个合约已经有 initializer 修饰符了
|
||
|
|
```
|
||
|
|
|
||
|
|
**实际问题**: 检查代码发现大部分合约已经使用了 `initializer` 修饰符。
|
||
|
|
但 Slither 可能是在检测 **proxy 合约的部署安全性**。
|
||
|
|
|
||
|
|
**额外保护**: 在实现合约的构造函数中添加 `_disableInitializers()`
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
/// @custom:oz-upgrades-unsafe-allow constructor
|
||
|
|
constructor() {
|
||
|
|
_disableInitializers();
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
这样可以防止实现合约被直接初始化。
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
### 2. ⚠️ 任意 from 地址的 transferFrom
|
||
|
|
|
||
|
|
**位置**: `YTPoolManager.sol` Line 158
|
||
|
|
|
||
|
|
**问题代码**:
|
||
|
|
```solidity
|
||
|
|
function _addLiquidity(
|
||
|
|
address _fundingAccount, // ⚠️ 可以是任意地址
|
||
|
|
address _account,
|
||
|
|
address _token,
|
||
|
|
uint256 _amount,
|
||
|
|
uint256 _minUsdy,
|
||
|
|
uint256 _minYtLP
|
||
|
|
) private returns (uint256) {
|
||
|
|
// ...
|
||
|
|
IERC20(_token).safeTransferFrom(_fundingAccount, ytVault, _amount); // ⚠️ 危险!
|
||
|
|
// ...
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
**风险**: 如果 `_fundingAccount` 可以被恶意控制,可能导致未授权的代币转移。
|
||
|
|
|
||
|
|
#### ✅ 修复方案
|
||
|
|
|
||
|
|
**方案 1: 限制调用者 (推荐)**
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
function _addLiquidity(
|
||
|
|
address _fundingAccount,
|
||
|
|
address _account,
|
||
|
|
address _token,
|
||
|
|
uint256 _amount,
|
||
|
|
uint256 _minUsdy,
|
||
|
|
uint256 _minYtLP
|
||
|
|
) private returns (uint256) {
|
||
|
|
if (_amount == 0) revert InvalidAmount();
|
||
|
|
|
||
|
|
// ✅ 添加: 确保只有授权的 handler 或用户本人可以使用其账户
|
||
|
|
if (_fundingAccount != msg.sender && !isHandler[msg.sender]) {
|
||
|
|
revert Forbidden();
|
||
|
|
}
|
||
|
|
|
||
|
|
uint256 aumInUsdy = getAumInUsdy(true);
|
||
|
|
uint256 ytLPSupply = IERC20(ytLP).totalSupply();
|
||
|
|
|
||
|
|
IERC20(_token).safeTransferFrom(_fundingAccount, ytVault, _amount);
|
||
|
|
// ... 后续逻辑
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
**方案 2: 使用白名单**
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// 添加状态变量
|
||
|
|
mapping(address => bool) public approvedFundingAccounts;
|
||
|
|
|
||
|
|
// 添加管理函数
|
||
|
|
function setApprovedFundingAccount(address _account, bool _approved) external onlyGov {
|
||
|
|
approvedFundingAccounts[_account] = _approved;
|
||
|
|
}
|
||
|
|
|
||
|
|
// 在 _addLiquidity 中检查
|
||
|
|
function _addLiquidity(...) private returns (uint256) {
|
||
|
|
if (!approvedFundingAccounts[_fundingAccount] && _fundingAccount != msg.sender) {
|
||
|
|
revert UnauthorizedFundingAccount();
|
||
|
|
}
|
||
|
|
// ...
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🟠 P1: 中危问题 (尽快修复)
|
||
|
|
|
||
|
|
### 3. 重入漏洞
|
||
|
|
|
||
|
|
**位置**: `YTVault.sol` Line 331-335
|
||
|
|
|
||
|
|
**问题代码**:
|
||
|
|
```solidity
|
||
|
|
function sellUSDY(address _token, address _receiver)
|
||
|
|
external
|
||
|
|
onlyPoolManager
|
||
|
|
nonReentrant // ✅ 已经有 nonReentrant,但顺序不对
|
||
|
|
notInEmergency
|
||
|
|
returns (uint256)
|
||
|
|
{
|
||
|
|
// ... 前面的逻辑
|
||
|
|
|
||
|
|
// ❌ 外部调用
|
||
|
|
IUSDY(usdy).burn(address(this), usdyAmount);
|
||
|
|
|
||
|
|
// ❌ 之后更新状态
|
||
|
|
IERC20(_token).safeTransfer(_receiver, amountOut);
|
||
|
|
_updateTokenBalance(_token); // 状态更新在外部调用后!
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
**分析**: 虽然已经有 `nonReentrant` 修饰符,但 Slither 仍然报告是因为:
|
||
|
|
1. `_updateTokenBalance` 在外部调用后执行
|
||
|
|
2. 如果 `_receiver` 是恶意合约,可能在 `safeTransfer` 回调中做些什么
|
||
|
|
|
||
|
|
#### ✅ 修复方案
|
||
|
|
|
||
|
|
**遵循 CEI 模式 (Checks-Effects-Interactions)**
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
function sellUSDY(address _token, address _receiver)
|
||
|
|
external
|
||
|
|
onlyPoolManager
|
||
|
|
nonReentrant
|
||
|
|
notInEmergency
|
||
|
|
returns (uint256)
|
||
|
|
{
|
||
|
|
if (!whitelistedTokens[_token]) revert TokenNotWhitelisted();
|
||
|
|
if (!isSwapEnabled) revert SwapDisabled();
|
||
|
|
|
||
|
|
uint256 usdyAmount = _transferIn(usdy);
|
||
|
|
if (usdyAmount == 0) revert InvalidAmount();
|
||
|
|
|
||
|
|
uint256 price = _getPrice(_token, true);
|
||
|
|
uint256 redemptionAmount = usdyAmount * PRICE_PRECISION / price;
|
||
|
|
redemptionAmount = _adjustForDecimals(redemptionAmount, usdy, _token);
|
||
|
|
if (redemptionAmount == 0) revert InvalidAmount();
|
||
|
|
|
||
|
|
uint256 feeBasisPoints = _getSwapFeeBasisPoints(usdy, _token, redemptionAmount);
|
||
|
|
uint256 amountOut = redemptionAmount * (BASIS_POINTS_DIVISOR - feeBasisPoints) / BASIS_POINTS_DIVISOR;
|
||
|
|
if (amountOut == 0) revert InvalidAmount();
|
||
|
|
if (poolAmounts[_token] < amountOut) revert InsufficientPool();
|
||
|
|
|
||
|
|
uint256 usdyAmountOut = amountOut * price / PRICE_PRECISION;
|
||
|
|
usdyAmountOut = _adjustForDecimals(usdyAmountOut, _token, usdy);
|
||
|
|
|
||
|
|
// ✅ 1. Effects: 先更新所有状态
|
||
|
|
_decreasePoolAmount(_token, amountOut);
|
||
|
|
_decreaseUsdyAmount(_token, usdyAmountOut);
|
||
|
|
|
||
|
|
// ✅ 预先更新 tokenBalance (模拟转出后的余额)
|
||
|
|
uint256 currentBalance = IERC20(_token).balanceOf(address(this));
|
||
|
|
tokenBalances[_token] = currentBalance - amountOut;
|
||
|
|
|
||
|
|
// ✅ 2. Interactions: 最后进行外部调用
|
||
|
|
IUSDY(usdy).burn(address(this), usdyAmount);
|
||
|
|
IERC20(_token).safeTransfer(_receiver, amountOut);
|
||
|
|
|
||
|
|
// ✅ 3. 验证最终余额 (可选,增强安全性)
|
||
|
|
uint256 finalBalance = IERC20(_token).balanceOf(address(this));
|
||
|
|
require(finalBalance == tokenBalances[_token], "Balance mismatch");
|
||
|
|
|
||
|
|
emit RemoveLiquidity(_receiver, _token, usdyAmount, amountOut);
|
||
|
|
|
||
|
|
return amountOut;
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
### 4. 先除后乘导致精度损失
|
||
|
|
|
||
|
|
**位置**: 多处
|
||
|
|
|
||
|
|
#### 问题 1: `Lending.sol` Line 343, 346
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前
|
||
|
|
uint256 collateralValueUSD = (collateralAmount * assetPrice) / assetScale;
|
||
|
|
uint256 discountedValue = (collateralValueUSD * assetConfig.liquidationFactor) / 1e18;
|
||
|
|
|
||
|
|
// ✅ 修复后
|
||
|
|
uint256 collateralValueUSD = (collateralAmount * assetPrice * assetConfig.liquidationFactor)
|
||
|
|
/ (assetScale * 1e18);
|
||
|
|
```
|
||
|
|
|
||
|
|
#### 问题 2: `Lending.sol` Line 470, 479
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前
|
||
|
|
uint256 assetPriceDiscounted = (assetPrice * (FACTOR_SCALE - discountFactor)) / FACTOR_SCALE;
|
||
|
|
uint256 result = (basePrice * baseAmount * assetScale) / (assetPriceDiscounted * baseScale);
|
||
|
|
|
||
|
|
// ✅ 修复后
|
||
|
|
uint256 result = (basePrice * baseAmount * assetScale * FACTOR_SCALE)
|
||
|
|
/ (assetPrice * (FACTOR_SCALE - discountFactor) * baseScale);
|
||
|
|
```
|
||
|
|
|
||
|
|
#### 问题 3: `YTVault.sol` Line 548, 552
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前
|
||
|
|
uint256 averageDiff = (initialDiff + nextDiff) / 2;
|
||
|
|
uint256 taxBps = _taxBasisPoints * averageDiff / targetAmount;
|
||
|
|
|
||
|
|
// ✅ 修复后
|
||
|
|
uint256 taxBps = _taxBasisPoints * (initialDiff + nextDiff) / (2 * targetAmount);
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🟡 P2: 低危/信息性问题
|
||
|
|
|
||
|
|
### 5. 缺少事件发射
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// YTPoolManager.sol Line 112
|
||
|
|
function setGov(address _gov) external onlyGov {
|
||
|
|
if (_gov == address(0)) revert InvalidAddress();
|
||
|
|
address oldGov = gov; // ✅ 添加
|
||
|
|
gov = _gov;
|
||
|
|
emit GovChanged(oldGov, _gov); // ✅ 添加事件
|
||
|
|
}
|
||
|
|
|
||
|
|
// ✅ 添加事件定义
|
||
|
|
event GovChanged(address indexed oldGov, address indexed newGov);
|
||
|
|
event PoolManagerChanged(address indexed oldManager, address indexed newManager);
|
||
|
|
event AumAdjustmentChanged(uint256 addition, uint256 deduction);
|
||
|
|
```
|
||
|
|
|
||
|
|
### 6. 缺少零地址检查
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// YTAssetVault.sol Line 176
|
||
|
|
function setManager(address _manager) external onlyFactory {
|
||
|
|
require(_manager != address(0), "Zero address"); // ✅ 添加检查
|
||
|
|
manager = _manager;
|
||
|
|
emit ManagerSet(_manager);
|
||
|
|
}
|
||
|
|
|
||
|
|
// YTPriceFeed.sol Line 80
|
||
|
|
function setWusdPriceSource(address _wusdPriceSource) external onlyGov {
|
||
|
|
require(_wusdPriceSource != address(0), "Zero address"); // ✅ 添加检查
|
||
|
|
wusdPriceSource = _wusdPriceSource;
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
### 7. 忽略返回值
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// YTRewardRouter.sol Line 117
|
||
|
|
// ❌ 修复前
|
||
|
|
IERC20(_token).approve(ytPoolManager, _amount);
|
||
|
|
|
||
|
|
// ✅ 修复后
|
||
|
|
bool success = IERC20(_token).approve(ytPoolManager, _amount);
|
||
|
|
require(success, "Approve failed");
|
||
|
|
|
||
|
|
// 或者使用 SafeERC20
|
||
|
|
IERC20(_token).safeApprove(ytPoolManager, _amount);
|
||
|
|
```
|
||
|
|
|
||
|
|
### 8. Solidity 版本统一
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// ❌ 修复前: 各个文件使用不同版本
|
||
|
|
pragma solidity ^0.8.0;
|
||
|
|
|
||
|
|
// ✅ 修复后: 统一使用
|
||
|
|
pragma solidity ^0.8.20;
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 📋 修复检查清单
|
||
|
|
|
||
|
|
### P0 - 高危问题
|
||
|
|
- [ ] 在所有实现合约添加 `_disableInitializers()` 到构造函数
|
||
|
|
- [ ] 修复 `YTPoolManager._addLiquidity` 的任意 from 问题
|
||
|
|
- [ ] 验证所有合约的 `initialize` 函数有 `initializer` 修饰符
|
||
|
|
|
||
|
|
### P1 - 中危问题
|
||
|
|
- [ ] 重构 `YTVault.sellUSDY` 遵循 CEI 模式
|
||
|
|
- [ ] 修复所有先除后乘的精度问题
|
||
|
|
- [ ] Lending._absorbInternal
|
||
|
|
- [ ] Lending.quoteCollateral
|
||
|
|
- [ ] YTVault.sellUSDY
|
||
|
|
- [ ] YTVault.swap
|
||
|
|
- [ ] YTVault.getFeeBasisPoints
|
||
|
|
|
||
|
|
### P2 - 低危问题
|
||
|
|
- [ ] 添加缺失的事件
|
||
|
|
- [ ] 添加零地址检查
|
||
|
|
- [ ] 修复忽略返回值的问题
|
||
|
|
- [ ] 统一 Solidity 版本到 ^0.8.20
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🧪 测试建议
|
||
|
|
|
||
|
|
### 1. 初始化安全测试
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
// test/security/InitializerTest.t.sol
|
||
|
|
function testCannotReinitialize() public {
|
||
|
|
vm.expectRevert();
|
||
|
|
factory.initialize(address(vault), 1000);
|
||
|
|
}
|
||
|
|
|
||
|
|
function testCannotInitializeImplementation() public {
|
||
|
|
YTAssetVault impl = new YTAssetVault();
|
||
|
|
vm.expectRevert();
|
||
|
|
impl.initialize("Test", "TST", address(this), 1000, address(0), 0, 0, 0);
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
### 2. 重入攻击测试
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
contract MaliciousReceiver {
|
||
|
|
YTVault public vault;
|
||
|
|
bool public attacked;
|
||
|
|
|
||
|
|
function attack() external {
|
||
|
|
// 尝试重入
|
||
|
|
vault.sellUSDY(token, address(this));
|
||
|
|
}
|
||
|
|
|
||
|
|
// ERC20 回调
|
||
|
|
function onERC20Received(...) external {
|
||
|
|
if (!attacked) {
|
||
|
|
attacked = true;
|
||
|
|
vault.sellUSDY(token, address(this)); // 应该失败
|
||
|
|
}
|
||
|
|
}
|
||
|
|
}
|
||
|
|
|
||
|
|
function testReentrancyProtection() public {
|
||
|
|
MaliciousReceiver attacker = new MaliciousReceiver(vault);
|
||
|
|
vm.expectRevert("ReentrancyGuard: reentrant call");
|
||
|
|
attacker.attack();
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
### 3. 精度损失测试
|
||
|
|
|
||
|
|
```solidity
|
||
|
|
function testMathPrecision() public {
|
||
|
|
uint256 price = 1e30;
|
||
|
|
uint256 amount = 1e18;
|
||
|
|
uint256 scale = 1e18;
|
||
|
|
|
||
|
|
// 测试先除后乘 vs 先乘后除的差异
|
||
|
|
uint256 result1 = (amount * price) / scale;
|
||
|
|
uint256 result2 = amount * (price / scale);
|
||
|
|
|
||
|
|
assertEq(result1, result2, "Precision loss detected");
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 📞 后续步骤
|
||
|
|
|
||
|
|
1. **立即修复 P0 问题**
|
||
|
|
2. **运行完整测试套件**
|
||
|
|
3. **重新运行 Slither 验证修复**
|
||
|
|
4. **考虑进行专业审计**
|
||
|
|
|
||
|
|
```bash
|
||
|
|
# 重新运行 Slither
|
||
|
|
slither . --exclude-informational --exclude-low
|
||
|
|
|
||
|
|
# 运行测试
|
||
|
|
forge test -vvv
|
||
|
|
|
||
|
|
# 生成覆盖率报告
|
||
|
|
forge coverage
|
||
|
|
```
|
||
|
|
|