diff --git a/src/IntuitionFeeProxy.sol b/src/IntuitionFeeProxy.sol index 35993d7..40f74ff 100644 --- a/src/IntuitionFeeProxy.sol +++ b/src/IntuitionFeeProxy.sol @@ -196,8 +196,8 @@ contract IntuitionFeeProxy { // ============ Proxy Functions (Payable) ============ /// @notice Create atoms with fee collection and deposit to receiver - /// @dev Receiver must have approved this proxy on MultiVault for DEPOSIT - /// @param receiver Address to receive the shares (the real user) + /// @dev Receiver must be msg.sender to prevent unauthorized deposits to arbitrary addresses + /// @param receiver Address to receive the shares (must be msg.sender) /// @param data Array of atom data (IPFS URIs as bytes) /// @param assets Array of deposit amounts for each atom (on top of creation cost) /// @param curveId Bonding curve ID for deposits (1 = linear, 2 = progressive) @@ -208,6 +208,9 @@ contract IntuitionFeeProxy { uint256[] calldata assets, uint256 curveId ) external payable returns (bytes32[] memory atomIds) { + if (receiver != msg.sender) { + revert Errors.IntuitionFeeProxy_ReceiverNotCaller(); + } if (data.length != assets.length) { revert Errors.IntuitionFeeProxy_WrongArrayLengths(); } @@ -255,8 +258,8 @@ contract IntuitionFeeProxy { } /// @notice Create triples with fee collection and deposit to receiver - /// @dev Receiver must have approved this proxy on MultiVault for DEPOSIT - /// @param receiver Address to receive the shares (the real user) + /// @dev Receiver must be msg.sender to prevent unauthorized deposits to arbitrary addresses + /// @param receiver Address to receive the shares (must be msg.sender) /// @param subjectIds Array of subject atom IDs /// @param predicateIds Array of predicate atom IDs /// @param objectIds Array of object atom IDs @@ -271,6 +274,9 @@ contract IntuitionFeeProxy { uint256[] calldata assets, uint256 curveId ) external payable returns (bytes32[] memory tripleIds) { + if (receiver != msg.sender) { + revert Errors.IntuitionFeeProxy_ReceiverNotCaller(); + } if (subjectIds.length != predicateIds.length || predicateIds.length != objectIds.length || objectIds.length != assets.length) { @@ -325,8 +331,9 @@ contract IntuitionFeeProxy { } /// @notice Deposit with fee collection - SAME SIGNATURE AS MULTIVAULT + /// @dev Receiver must be msg.sender to prevent unauthorized deposits to arbitrary addresses /// @dev Fee is calculated from msg.value using inverse formula - /// @param receiver Address to receive shares + /// @param receiver Address to receive shares (must be msg.sender) /// @param termId Vault ID (atom or triple) /// @param curveId Bonding curve ID /// @param minShares Minimum shares expected @@ -337,6 +344,10 @@ contract IntuitionFeeProxy { uint256 curveId, uint256 minShares ) external payable returns (uint256 shares) { + // Receiver must be the caller to prevent depositing to arbitrary addresses + if (receiver != msg.sender) { + revert Errors.IntuitionFeeProxy_ReceiverNotCaller(); + } // Must send more than just the fixed fee if (msg.value <= depositFixedFee) { revert Errors.IntuitionFeeProxy_InsufficientValue(); @@ -371,7 +382,8 @@ contract IntuitionFeeProxy { } /// @notice Batch deposit with fee collection - /// @param receiver Address to receive shares + /// @dev Receiver must be msg.sender to prevent unauthorized deposits to arbitrary addresses + /// @param receiver Address to receive shares (must be msg.sender) /// @param termIds Array of vault IDs /// @param curveIds Array of curve IDs /// @param assets Array of deposit amounts @@ -384,6 +396,10 @@ contract IntuitionFeeProxy { uint256[] calldata assets, uint256[] calldata minShares ) external payable returns (uint256[] memory shares) { + // Receiver must be the caller to prevent depositing to arbitrary addresses + if (receiver != msg.sender) { + revert Errors.IntuitionFeeProxy_ReceiverNotCaller(); + } if (termIds.length != curveIds.length || curveIds.length != assets.length || assets.length != minShares.length) { diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 839d842..c5071ed 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -25,6 +25,9 @@ library Errors { /// @notice Zero address provided where not allowed error IntuitionFeeProxy_ZeroAddress(); + /// @notice Receiver address does not match the caller (msg.sender) + error IntuitionFeeProxy_ReceiverNotCaller(); + /// @notice Fee percentage exceeds maximum allowed (100%) error IntuitionFeeProxy_FeePercentageTooHigh(); } diff --git a/test/IntuitionFeeProxy.test.ts b/test/IntuitionFeeProxy.test.ts index 9365c5e..c58c2bb 100644 --- a/test/IntuitionFeeProxy.test.ts +++ b/test/IntuitionFeeProxy.test.ts @@ -398,6 +398,92 @@ describe("IntuitionFeeProxy", function () { }); }); + describe("Receiver Validation (Issue #1)", function () { + it("Should revert deposit when receiver != msg.sender", async function () { + const { proxy, user, nonAdmin } = await loadFixture(deployFixture); + const termId = ethers.zeroPadValue("0x01", 32); + // user calls deposit but specifies nonAdmin as receiver + await expect( + proxy.connect(user).deposit(nonAdmin.address, termId, 1n, 0n, { value: ethers.parseEther("1") }) + ).to.be.revertedWithCustomError(proxy, "IntuitionFeeProxy_ReceiverNotCaller"); + }); + + it("Should allow deposit when receiver == msg.sender", async function () { + const { proxy, user } = await loadFixture(deployFixture); + const termId = ethers.zeroPadValue("0x01", 32); + // user calls deposit with their own address — should succeed + await expect( + proxy.connect(user).deposit(user.address, termId, 1n, 0n, { value: ethers.parseEther("1") }) + ).to.not.be.reverted; + }); + + it("Should revert createAtoms when receiver != msg.sender", async function () { + const { proxy, user, nonAdmin } = await loadFixture(deployFixture); + const data = [ethers.toUtf8Bytes("ipfs://test1")]; + const assets = [0n]; + // user calls createAtoms but specifies nonAdmin as receiver + await expect( + proxy.connect(user).createAtoms(nonAdmin.address, data, assets, 1n, { value: ethers.parseEther("1") }) + ).to.be.revertedWithCustomError(proxy, "IntuitionFeeProxy_ReceiverNotCaller"); + }); + + it("Should allow createAtoms when receiver == msg.sender", async function () { + const { proxy, user } = await loadFixture(deployFixture); + const data = [ethers.toUtf8Bytes("ipfs://test1")]; + const assets = [0n]; + await expect( + proxy.connect(user).createAtoms(user.address, data, assets, 1n, { value: ethers.parseEther("1") }) + ).to.not.be.reverted; + }); + + it("Should revert createTriples when receiver != msg.sender", async function () { + const { proxy, user, nonAdmin } = await loadFixture(deployFixture); + const subjectIds = [ethers.zeroPadValue("0x01", 32)]; + const predicateIds = [ethers.zeroPadValue("0x02", 32)]; + const objectIds = [ethers.zeroPadValue("0x03", 32)]; + const assets = [0n]; + await expect( + proxy.connect(user).createTriples(nonAdmin.address, subjectIds, predicateIds, objectIds, assets, 1n, { value: ethers.parseEther("1") }) + ).to.be.revertedWithCustomError(proxy, "IntuitionFeeProxy_ReceiverNotCaller"); + }); + + it("Should allow createTriples when receiver == msg.sender", async function () { + const { proxy, user } = await loadFixture(deployFixture); + const subjectIds = [ethers.zeroPadValue("0x01", 32)]; + const predicateIds = [ethers.zeroPadValue("0x02", 32)]; + const objectIds = [ethers.zeroPadValue("0x03", 32)]; + const assets = [0n]; + await expect( + proxy.connect(user).createTriples(user.address, subjectIds, predicateIds, objectIds, assets, 1n, { value: ethers.parseEther("1") }) + ).to.not.be.reverted; + }); + + it("Should revert depositBatch when receiver != msg.sender", async function () { + const { proxy, user, nonAdmin } = await loadFixture(deployFixture); + const termIds = [ethers.zeroPadValue("0x01", 32), ethers.zeroPadValue("0x02", 32)]; + const curveIds = [1n, 1n]; + const assets = [ethers.parseEther("5"), ethers.parseEther("5")]; + const minShares = [0n, 0n]; + await expect( + proxy.connect(user).depositBatch(nonAdmin.address, termIds, curveIds, assets, minShares, { value: ethers.parseEther("20") }) + ).to.be.revertedWithCustomError(proxy, "IntuitionFeeProxy_ReceiverNotCaller"); + }); + + it("Should allow depositBatch when receiver == msg.sender", async function () { + const { proxy, user } = await loadFixture(deployFixture); + const termIds = [ethers.zeroPadValue("0x01", 32), ethers.zeroPadValue("0x02", 32)]; + const curveIds = [1n, 1n]; + const assets = [ethers.parseEther("5"), ethers.parseEther("5")]; + const minShares = [0n, 0n]; + const totalDeposit = ethers.parseEther("10"); + const fee = await proxy.calculateDepositFee(2n, totalDeposit); + const totalRequired = totalDeposit + fee; + await expect( + proxy.connect(user).depositBatch(user.address, termIds, curveIds, assets, minShares, { value: totalRequired }) + ).to.not.be.reverted; + }); + }); + describe("View Functions (Passthrough)", function () { it("Should return atom cost from MultiVault", async function () { const { proxy, mockMultiVault } = await loadFixture(deployFixture);