Skip to content

Commit 9c69cfe

Browse files
source-originsource-origin
committed
fix(l5): close unauthorized-privileged + reentrancy holes found in self-audit (C1-C4)
A full self-audit of all 8 contracts (the suite was 62/62 green yet none of these paths were exercised) found privileged functions whose auth was stubbed out and a value-transfer reentrancy. Each fix is fail-closed and covered by a named test. - C1 AgentEscrow.resolveDispute: the "contribution clock" require was commented out, so ANYONE could resolve any disputed escrow and choose payeeWins, sending the escrowed amount + dispute bond to a party of their choosing. Now onlyOwner (TODO: hand to the contribution clock) + nonReentrant. - C2 AgentEscrow.finalizeSettlement: no reentrancy guard and channel state was set AFTER the value transfers, allowing a reentrant double-pay. Now nonReentrant + checks-effects-interactions (state Closed before transfers). - C3 AgentIdentity.slashAgent / suspendAgent / upgradeVerification / recordRevenue: all four privileged functions had their auth require commented out, so anyone could slash/suspend/upgrade any agent or inflate any agent totalRevenue (which feeds reputation, leaderboard and CreditScore). Now onlyOwner. - C4 AgentEscrow.setDisputeBond: no access control (the contract did not even inherit Ownable). Now onlyOwner. AgentEscrow now inherits Ownable + ReentrancyGuard (constructor Ownable(msg.sender)). Regression: test/L5AccessControl.t.sol adds 8 named tests (non-owner reverts for each privileged fn, owner succeeds, and a reentrancy receiver is paid exactly once). forge fmt clean; forge test 70/70 (was 62).
1 parent d89b913 commit 9c69cfe

3 files changed

Lines changed: 258 additions & 21 deletions

File tree

‎src/AgentEscrow.sol‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
pragma solidity ^0.8.28;
2424

2525
import "./AgentAgreement.sol";
26+
import "@openzeppelin/contracts/access/Ownable.sol";
27+
import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
2628

2729
/**
2830
* @title AgentEscrow
2931
* @notice AI Agent 之间的链上托管支付合?+
3032
* @dev 与AgentAgreement紧密集成，通过agreementId关联
3133
*/
32-
contract AgentEscrow {
34+
contract AgentEscrow is Ownable, ReentrancyGuard {
3335
// ══════════════════════════════════════════════════
3436
// ENUMS
3537
// ══════════════════════════════════════════════════
@@ -181,7 +183,7 @@ contract AgentEscrow {
181183
// ══════════════════════════════════════════════════
182184
// CONSTRUCTOR
183185
// ══════════════════════════════════════════════════
184-
constructor(address _agentAgreement) {
186+
constructor(address _agentAgreement) Ownable(msg.sender) {
185187
require(_agentAgreement != address(0), "Escrow: zero address");
186188
agentAgreement = AgentAgreement(_agentAgreement);
187189
}
@@ -352,13 +354,15 @@ contract AgentEscrow {
352354
// / @param payeeWins true=Provider获胜得全款，false=Consumer获胜退
353355
function resolveDispute(bytes32 escrowId, bool payeeWins)
354356
external
357+
onlyOwner
358+
nonReentrant
355359
escrowExists(escrowId)
356360
onlyState(escrowId, EscrowState.Disputed)
357361
{
358362
Escrow storage esc = escrows[escrowId];
359363

360-
// 未来：由贡献时钟验证调用权限
361-
// require(msg.sender == address(contributionClock), "Escrow: only contribution clock");
364+
// 止血锁：贡献时钟尚未部署，裁决者暂由 owner 担任（严格严于「任何人可调」）。
365+
// TODO(接线): 贡献时钟合约上线后改为 require(msg.sender == address(contributionClock), "Escrow: only contribution clock");
362366

363367
uint256 escrowAmount = esc.amount;
364368
uint256 bondAmount = esc.disputeBond;
@@ -407,7 +411,7 @@ contract AgentEscrow {
407411
// ══════════════════════════════════════════════════
408412
// ADMIN
409413
// ══════════════════════════════════════════════════
410-
function setDisputeBond(uint256 newBps) external {
414+
function setDisputeBond(uint256 newBps) external onlyOwner {
411415
require(newBps <= MAX_DISPUTE_BOND_BPS, "Escrow: bond too high");
412416
disputeBondBps = newBps;
413417
}
@@ -487,14 +491,18 @@ contract AgentEscrow {
487491
}
488492

489493
/// @notice 挑战期过后完成结算
490-
function finalizeSettlement(bytes32 channelId) external {
494+
function finalizeSettlement(bytes32 channelId) external nonReentrant {
491495
PaymentChannel storage ch = channels[channelId];
492496
require(ch.state == ChannelState.Settling, "Channel: not settling");
493497
require(block.timestamp >= ch.settlingAt + challengePeriod, "Channel: challenge period not over");
494498

495499
uint256 cumulativeAmount = ch.pendingAmount;
496500
uint256 senderRefund = ch.balance - cumulativeAmount;
497501

502+
// Checks-Effects-Interactions：先落状态再转账（防重入双重支付）
503+
ch.balance = 0;
504+
ch.state = ChannelState.Closed;
505+
498506
if (cumulativeAmount > 0) {
499507
(bool paid,) = ch.receiver.call{value: cumulativeAmount}("");
500508
require(paid, "Channel: payment failed");
@@ -504,9 +512,6 @@ contract AgentEscrow {
504512
require(refunded, "Channel: refund failed");
505513
}
506514

507-
ch.balance = 0;
508-
ch.state = ChannelState.Closed;
509-
510515
emit ChannelSettled(channelId, cumulativeAmount, block.timestamp);
511516
}
512517

‎src/AgentIdentity.sol‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -574,10 +574,14 @@ contract AgentIdentity is ERC721, ERC721URIStorage, Ownable {
574574
// CORE: VERIFICATION
575575
// ═══════════════════════════════════════════════════
576576

577-
/// @notice 提升验证等级（由贡献时钟合约调用）
578-
function upgradeVerification(address agentAddr, VerificationLevel newLevel) external agentExists(agentAddr) {
579-
// 未来：由贡献时钟或DAO治理合约调用
580-
// require(msg.sender == contributionClock || msg.sender == daoGovernance);
577+
/// @notice 提升验证等级（止血锁：暂由 owner 调用）
578+
function upgradeVerification(address agentAddr, VerificationLevel newLevel)
579+
external
580+
onlyOwner
581+
agentExists(agentAddr)
582+
{
583+
// 止血锁：贡献时钟/DAO 尚未部署，暂由 owner 担任（严格严于「任何人可调」）。
584+
// TODO(接线): 贡献时钟或 DAO 治理合约上线后改为 require(msg.sender == contributionClock || msg.sender == daoGovernance);
581585
require(uint8(newLevel) > uint8(agents[agentAddr].verification), "Identity: cannot downgrade");
582586

583587
agents[agentAddr].verification = newLevel;
@@ -590,8 +594,8 @@ contract AgentIdentity is ERC721, ERC721URIStorage, Ownable {
590594
// CORE: REVENUE TRACKING
591595
// ═══════════════════════════════════════════════════
592596

593-
/// @notice 记录Agent收入（由AgentEscrow合约调用）
594-
function recordRevenue(address agentAddr, uint256 amount) external agentExists(agentAddr) {
597+
/// @notice 记录Agent收入（止血锁：暂由 owner 调用）
598+
function recordRevenue(address agentAddr, uint256 amount) external onlyOwner agentExists(agentAddr) {
595599
Agent storage a = agents[agentAddr];
596600
a.totalRevenue += amount;
597601
a.totalTransactions += 1;
@@ -656,9 +660,14 @@ contract AgentIdentity is ERC721, ERC721URIStorage, Ownable {
656660
// CORE: PENALTY
657661
// ═══════════════════════════════════════════════════
658662

659-
/// @notice 暂停Agent（由仲裁系统调用）
660-
function suspendAgent(address agentAddr, string calldata reason, uint256 days_) external agentExists(agentAddr) {
661-
// 未来：由贡献时钟或DAO调用
663+
/// @notice 暂停Agent（止血锁：暂由 owner 调用）
664+
function suspendAgent(address agentAddr, string calldata reason, uint256 days_)
665+
external
666+
onlyOwner
667+
agentExists(agentAddr)
668+
{
669+
// 止血锁：贡献时钟/DAO 尚未部署，暂由 owner 担任。
670+
// TODO(接线): 贡献时钟或 DAO 上线后改为 require(msg.sender == contributionClock || msg.sender == daoGovernance);
662671
Agent storage a = agents[agentAddr];
663672
a.status = AgentStatus.Suspended;
664673
a.updatedAt = block.timestamp;
@@ -671,9 +680,10 @@ contract AgentIdentity is ERC721, ERC721URIStorage, Ownable {
671680
emit AgentSuspended(agentAddr, reason, block.timestamp + days_ * 1 days, block.timestamp);
672681
}
673682

674-
/// @notice 罚没Agent（永久禁止）
675-
function slashAgent(address agentAddr, string calldata reason) external agentExists(agentAddr) {
676-
// 未来：由贡献时钟调用
683+
/// @notice 罚没Agent（永久禁止）（止血锁：暂由 owner 调用）
684+
function slashAgent(address agentAddr, string calldata reason) external onlyOwner agentExists(agentAddr) {
685+
// 止血锁：贡献时钟尚未部署，暂由 owner 担任。
686+
// TODO(接线): 贡献时钟上线后改为 require(msg.sender == contributionClock);
677687
Agent storage a = agents[agentAddr];
678688
a.status = AgentStatus.Slashed;
679689
a.verification = VerificationLevel.None;

‎test/L5AccessControl.t.sol‎

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
// SPDX-License-Identifier: ORIGIN-1.0
2+
pragma solidity ^0.8.28;
3+
4+
import {Test} from "forge-std/Test.sol";
5+
import {AgentIdentity} from "src/AgentIdentity.sol";
6+
import {AgentAgreement} from "src/AgentAgreement.sol";
7+
import {AgentEscrow} from "src/AgentEscrow.sol";
8+
9+
/// @title L5 访问控制止血包 · 回归测试
10+
/// @notice 锁定 2026-09-25 自查审计发现的 C 级问题：
11+
/// C1 resolveDispute 未授权、C2 finalizeSettlement 重入、
12+
/// C3 AgentIdentity 特权函数未授权、C4 setDisputeBond 未授权。
13+
/// @dev 测试合约即部署者 = owner；alice/bob/carol/mallory 为非 owner。
14+
contract L5AccessControlTest is Test {
15+
AgentIdentity public identity;
16+
AgentAgreement public agreement;
17+
AgentEscrow public escrow;
18+
19+
uint256 public kAlice = 0xA11CE;
20+
uint256 public kBob = 0xB0B;
21+
uint256 public kMallory = uint256(keccak256("mallory"));
22+
23+
address public alice;
24+
address public bob;
25+
address public mallory;
26+
27+
bytes32 public agreementId;
28+
29+
function setUp() public {
30+
alice = vm.addr(kAlice);
31+
bob = vm.addr(kBob);
32+
mallory = vm.addr(kMallory);
33+
34+
vm.deal(alice, 100 ether);
35+
vm.deal(bob, 100 ether);
36+
vm.deal(mallory, 100 ether);
37+
38+
identity = new AgentIdentity();
39+
agreement = new AgentAgreement();
40+
escrow = new AgentEscrow(address(agreement));
41+
}
42+
43+
// ─────────────────────────────────────────────
44+
// helpers
45+
// ─────────────────────────────────────────────
46+
47+
function _registerAlice() internal {
48+
vm.prank(alice);
49+
identity.registerAgent{value: 0.01 ether}("alice_trader", "Alice Trader", "quant bot", "trading", "ipfs://a");
50+
}
51+
52+
function _sign(bytes32 _agreementHash, uint256 _key) internal view returns (bytes memory) {
53+
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", agreement.domainSeparator(), _agreementHash));
54+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(_key, digest);
55+
return abi.encodePacked(r, s, v);
56+
}
57+
58+
function _executedAgreement() internal {
59+
AgentAgreement.AgentParty[] memory parties = new AgentAgreement.AgentParty[](2);
60+
parties[0] = AgentAgreement.AgentParty({
61+
agentAddr: alice, role: AgentAgreement.PartyRole.Consumer, signature: new bytes(0), signedAt: 0
62+
});
63+
parties[1] = AgentAgreement.AgentParty({
64+
agentAddr: bob, role: AgentAgreement.PartyRole.Provider, signature: new bytes(0), signedAt: 0
65+
});
66+
67+
AgentAgreement.SettlementTerm[] memory terms = new AgentAgreement.SettlementTerm[](1);
68+
terms[0] = AgentAgreement.SettlementTerm({
69+
termId: bytes32(0),
70+
termType: AgentAgreement.TermType.Payment,
71+
description: "execute trade",
72+
value: 5 ether,
73+
dueDate: block.timestamp + 7 days,
74+
completed: false,
75+
completedAt: 0
76+
});
77+
78+
vm.prank(alice);
79+
agreementId = agreement.createAgreement("trade-1", "BTC trade", parties, terms, 5 ether);
80+
vm.prank(alice);
81+
agreement.propose(agreementId);
82+
83+
bytes32 hash = agreement.getAgreement(agreementId).agreementHash;
84+
bytes memory sigAlice = _sign(hash, kAlice);
85+
bytes memory sigBob = _sign(hash, kBob);
86+
vm.prank(alice);
87+
agreement.signAgreement(agreementId, sigAlice);
88+
vm.prank(bob);
89+
agreement.signAgreement(agreementId, sigBob);
90+
}
91+
92+
/// @notice 走到 Disputed 状态：Funded → Verified → Disputed
93+
function _disputedEscrow() internal returns (bytes32 escrowId) {
94+
_executedAgreement();
95+
vm.prank(alice);
96+
escrowId = escrow.createAndFund{value: 5 ether}(agreementId, bob, block.timestamp + 7 days);
97+
98+
vm.prank(bob);
99+
escrow.verifyDelivery(escrowId, "ipfs://proof");
100+
101+
uint256 bond = (5 ether * escrow.disputeBondBps()) / 10000;
102+
vm.prank(alice);
103+
escrow.dispute{value: bond}(escrowId);
104+
require(uint8(escrow.getEscrow(escrowId).state) == uint8(AgentEscrow.EscrowState.Disputed), "not disputed");
105+
}
106+
107+
// ─────────────────────────────────────────────
108+
// C1 · resolveDispute 仅 owner
109+
// ─────────────────────────────────────────────
110+
111+
function test_C1_ResolveDispute_NonOwner_Reverts() public {
112+
bytes32 escrowId = _disputedEscrow();
113+
vm.prank(mallory);
114+
vm.expectRevert();
115+
escrow.resolveDispute(escrowId, true);
116+
assertEq(uint8(escrow.getEscrow(escrowId).state), uint8(AgentEscrow.EscrowState.Disputed));
117+
}
118+
119+
function test_C1_ResolveDispute_Owner_Succeeds() public {
120+
bytes32 escrowId = _disputedEscrow();
121+
uint256 bobBefore = bob.balance;
122+
escrow.resolveDispute(escrowId, true); // 测试合约 = owner
123+
assertEq(uint8(escrow.getEscrow(escrowId).state), uint8(AgentEscrow.EscrowState.Released));
124+
assertEq(bob.balance, bobBefore + 5 ether + 0.05 ether); // 托管 + 押金
125+
}
126+
127+
// ─────────────────────────────────────────────
128+
// C4 · setDisputeBond 仅 owner
129+
// ─────────────────────────────────────────────
130+
131+
function test_C4_SetDisputeBond_NonOwner_Reverts() public {
132+
vm.prank(mallory);
133+
vm.expectRevert();
134+
escrow.setDisputeBond(500);
135+
assertEq(escrow.disputeBondBps(), 100);
136+
}
137+
138+
// ─────────────────────────────────────────────
139+
// C3 · AgentIdentity 特权函数仅 owner
140+
// ─────────────────────────────────────────────
141+
142+
function test_C3_SlashAgent_NonOwner_Reverts() public {
143+
_registerAlice();
144+
vm.prank(mallory);
145+
vm.expectRevert();
146+
identity.slashAgent(alice, "malicious");
147+
assertEq(uint8(identity.getAgentStatus(alice)), uint8(AgentIdentity.AgentStatus.Active));
148+
}
149+
150+
function test_C3_SuspendAgent_NonOwner_Reverts() public {
151+
_registerAlice();
152+
vm.prank(mallory);
153+
vm.expectRevert();
154+
identity.suspendAgent(alice, "malicious", 7);
155+
assertEq(uint8(identity.getAgentStatus(alice)), uint8(AgentIdentity.AgentStatus.Active));
156+
}
157+
158+
function test_C3_UpgradeVerification_NonOwner_Reverts() public {
159+
_registerAlice();
160+
vm.prank(mallory);
161+
vm.expectRevert();
162+
identity.upgradeVerification(alice, AgentIdentity.VerificationLevel.Trusted);
163+
assertEq(uint8(identity.getAgent(alice).verification), uint8(AgentIdentity.VerificationLevel.Basic));
164+
}
165+
166+
function test_C3_RecordRevenue_NonOwner_Reverts() public {
167+
_registerAlice();
168+
uint256 before = identity.getAgent(alice).totalRevenue;
169+
vm.prank(mallory);
170+
vm.expectRevert();
171+
identity.recordRevenue(alice, 1000 ether);
172+
assertEq(identity.getAgent(alice).totalRevenue, before);
173+
}
174+
175+
// ─────────────────────────────────────────────
176+
// C2 · finalizeSettlement 重入被阻断
177+
// ─────────────────────────────────────────────
178+
179+
function test_C2_FinalizeSettlement_ReentrancyBlocked() public {
180+
ReentrantReceiver rr = new ReentrantReceiver(escrow);
181+
182+
vm.prank(alice);
183+
bytes32 cid = escrow.openChannel{value: 5 ether}(address(rr));
184+
rr.arm(cid);
185+
186+
// sender(alice) 签名，结算 1 ether
187+
bytes32 inner = keccak256(abi.encode(cid, uint256(1), uint256(1 ether)));
188+
bytes32 digest = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", inner));
189+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(kAlice, digest);
190+
escrow.settleChannel(cid, 1, 1 ether, abi.encodePacked(r, s, v));
191+
192+
vm.warp(block.timestamp + 8 days); // 越过挑战期
193+
escrow.finalizeSettlement(cid);
194+
195+
// 只付一次：receiver 收 1 ether，sender 退回 4 ether
196+
assertEq(address(rr).balance, 1 ether);
197+
assertEq(alice.balance, 100 ether - 5 ether + 4 ether);
198+
assertEq(rr.hits(), 1, "receiver must be paid exactly once");
199+
}
200+
}
201+
202+
/// @dev 恶意收款方：收到款时尝试重入 finalizeSettlement
203+
contract ReentrantReceiver {
204+
AgentEscrow public escrow;
205+
bytes32 public cid;
206+
uint256 public hits;
207+
208+
constructor(AgentEscrow _escrow) {
209+
escrow = _escrow;
210+
}
211+
212+
function arm(bytes32 _cid) external {
213+
cid = _cid;
214+
}
215+
216+
receive() external payable {
217+
hits++;
218+
if (hits == 1) {
219+
try escrow.finalizeSettlement(cid) {} catch {}
220+
}
221+
}
222+
}

0 commit comments

Comments
 (0)