Skip to content

fix: validate receiver is msg.sender in all deposit/creation functions (Issue #1) #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
korpo1337 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: validate receiver is msg.sender in all deposit/creation functions (Issue #1) #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/IntuitionFeeProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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();
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Errors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
86 changes: 86 additions & 0 deletions test/IntuitionFeeProxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down