Files
assetxContracts/SECURITY_FIX_GUIDE.md
2025-12-23 14:05:41 +08:00

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
```