chore: audit finding "Fee Address Is Using transfer" by yomarion · Pull Request #1739 · RequestNetwork/requestNetwork

yomarion · 2026-06-17T13:50:40Z

Description of the changes

Addressing the finding [Medium] Fee Address Is Using transfer from Creed DAO, in BatchConversionPayments only.

When native funds are transferred, the EthereumFeeProxy and BatchConversionPayments smart
contracts use the transfer function to transfer the native fees. transfer allocates a fixed 2300 gas
stipend, which is subject to future changes and is not considered reliable.

To avoid disrupting the system across gas-related protocol changes, we recommend using external calls to transfer the funds, as it is already done for recipient and caller transfers in the above functions. Additionally, we recommend checking that the fee amount is non-zero before attempting a transfer to save gas and avoid unnecessary actions.

greptile-apps · 2026-06-17T13:52:54Z

Greptile Summary

This PR addresses the Creed DAO audit finding by replacing feeAddress.transfer(batchFeeToPay) with a low-level call pattern (plus a non-zero guard) in both _batchNativeConversionPayments and _batchNativePayments, removing reliance on the 2300-gas stipend.

BatchConversionPayments.sol: transfer replaced with payable(feeAddress).call{value: batchFeeToPay}(''); the if (batchFeeToPay > 0) guard avoids unnecessary zero-value calls, matching the audit recommendation.
BatchNoConversionPayments.sol: Same pattern applied to _batchNativePayments; however, replacing transfer with an unbounded call introduces a reentrancy window — a malicious feeAddress contract can re-enter batchNativePayments with an empty request list, triggering the sendback on line 210 with msg.sender == feeAddress and draining the payer's excess-ETH refund before the outer frame completes.

Confidence Score: 3/5

Both changed functions now use unbounded low-level call for fee transfers without reentrancy protection; a malicious feeAddress can re-enter and drain the payer's ETH refund in either contract.

Replacing transfer with call correctly removes the 2300-gas stipend dependency, but neither function has a reentrancy guard. The new call in _batchNativePayments opens a path where a caller-supplied feeAddress re-enters with an empty request list and, via the sendback, receives all remaining contract balance instead of the payer. The same structural concern exists in _batchNativeConversionPayments. Both public entry points (batchNativePayments, batchNativeConversionPayments) are missing nonReentrant protection.

BatchNoConversionPayments.sol lines 203–212 and BatchConversionPayments.sol lines 312–319 — both sendback-after-fee-call sequences need reentrancy guards before merging.

Important Files Changed

Filename	Overview
packages/smart-contracts/src/contracts/BatchConversionPayments.sol	Replaces `feeAddress.transfer(batchFeeToPay)` with a guarded low-level `call` and adds a non-zero check, correctly addressing the gas-stipend audit finding; same reentrancy window exists as in the sibling function (already discussed).
packages/smart-contracts/src/contracts/BatchNoConversionPayments.sol	Replaces `feeAddress.transfer(amount)` with a guarded low-level `call`; the new unbounded `call` opens a reentrancy path where a malicious `feeAddress` can drain the payer's ETH refund via the sendback on line 210.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Payer as Payer (msg.sender)
    participant Batch as BatchNoConversionPayments
    participant Proxy as NativeFeeProxy
    participant FeeAddr as feeAddress (malicious)

    Payer->>Batch: "batchNativePayments{value: X}(requests, feeAddress)"
    Batch->>Proxy: "transferWithReferenceAndFee{value: amt+fee}(...)"
    Proxy-->>Batch: (leftover ETH returned)
    Note over Batch: amount = batchFee * sum(requestAmounts) / denom
    Batch->>FeeAddr: "call{value: amount}('') [NEW — replaces transfer]"
    activate FeeAddr
    FeeAddr->>Batch: "batchNativePayments{value:0}([], feeAddress)"
    Note over Batch: Empty loop → amount=0, fee step skipped
    Batch->>FeeAddr: "call{value: address(this).balance}('') [sendback to msg.sender=feeAddress]"
    FeeAddr-->>Batch: ok (payer refund drained)
    deactivate FeeAddr
    Note over Batch: address(this).balance == 0
    Batch->>Payer: sendback call → 0 ETH sent (refund stolen)

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Payer as Payer (msg.sender)
    participant Batch as BatchNoConversionPayments
    participant Proxy as NativeFeeProxy
    participant FeeAddr as feeAddress (malicious)

    Payer->>Batch: "batchNativePayments{value: X}(requests, feeAddress)"
    Batch->>Proxy: "transferWithReferenceAndFee{value: amt+fee}(...)"
    Proxy-->>Batch: (leftover ETH returned)
    Note over Batch: amount = batchFee * sum(requestAmounts) / denom
    Batch->>FeeAddr: "call{value: amount}('') [NEW — replaces transfer]"
    activate FeeAddr
    FeeAddr->>Batch: "batchNativePayments{value:0}([], feeAddress)"
    Note over Batch: Empty loop → amount=0, fee step skipped
    Batch->>FeeAddr: "call{value: address(this).balance}('') [sendback to msg.sender=feeAddress]"
    FeeAddr-->>Batch: ok (payer refund drained)
    deactivate FeeAddr
    Note over Batch: address(this).balance == 0
    Batch->>Payer: sendback call → 0 ETH sent (refund stolen)

_{Reviews (4): Last reviewed commit: "Update packages/smart-contracts/src/cont..." | Re-trigger Greptile}

github-actions · 2026-06-17T13:55:50Z

❌ Echidna Fuzzing Results

Mode: ( test sequences)
Status: Property Violations Found

Property Test Results

Status	Count
✅ Passed	0
❌ Failed	0
Total	0
Pass Rate	0%

📄 Full report and corpus available in workflow artifacts.

ℹ️ About Echidna Fuzzing

Echidna is a property-based fuzzer that generates random sequences of transactions
to test invariants (properties that should always hold true).

Properties tested:

Fee calculation bounds
Access control enforcement
Amount constraints
No duplicate payments
Zero address validation
Integer overflow protection

github-actions · 2026-06-17T13:56:30Z

⚠️ Slither Security Analysis

Status: Issues Found

Findings Summary

Severity	Count	Status
✅ High	0	Pass
✅ Medium	0	Pass
🔵 Low	0	Info
ℹ️ Informational	0	Info

📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.

github-actions · 2026-06-17T14:00:08Z

⚠️ Slither Security Analysis

Status: Issues Found

Findings Summary

Severity	Count	Status
✅ High	0	Pass
✅ Medium	0	Pass
🔵 Low	0	Info
ℹ️ Informational	0	Info

📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.

github-actions · 2026-06-17T14:01:47Z

⚠️ Slither Security Analysis

Status: Issues Found

Findings Summary

Severity	Count	Status
✅ High	0	Pass
✅ Medium	0	Pass
🔵 Low	0	Info
ℹ️ Informational	0	Info

📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.

github-actions · 2026-06-17T14:04:13Z

❌ Echidna Fuzzing Results

Mode: ( test sequences)
Status: Property Violations Found

Property Test Results

Status	Count
✅ Passed	0
❌ Failed	0
Total	0
Pass Rate	0%

📄 Full report and corpus available in workflow artifacts.

ℹ️ About Echidna Fuzzing

Echidna is a property-based fuzzer that generates random sequences of transactions
to test invariants (properties that should always hold true).

Properties tested:

Fee calculation bounds
Access control enforcement
Amount constraints
No duplicate payments
Zero address validation
Integer overflow protection

…ts.sol Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

github-actions · 2026-06-17T14:18:52Z

❌ Echidna Fuzzing Results

Mode: ( test sequences)
Status: Property Violations Found

Property Test Results

Status	Count
✅ Passed	0
❌ Failed	0
Total	0
Pass Rate	0%

📄 Full report and corpus available in workflow artifacts.

ℹ️ About Echidna Fuzzing

Echidna is a property-based fuzzer that generates random sequences of transactions
to test invariants (properties that should always hold true).

Properties tested:

Fee calculation bounds
Access control enforcement
Amount constraints
No duplicate payments
Zero address validation
Integer overflow protection

github-actions · 2026-06-17T14:19:17Z

✅ Echidna Fuzzing Results

Mode: ci (50000 test sequences)
Status: All Properties Passed

Property Test Results

Status	Count
✅ Passed	16
❌ Failed	0
Total	16
Pass Rate	100.0%

📄 Full report and corpus available in workflow artifacts.

ℹ️ About Echidna Fuzzing

Echidna is a property-based fuzzer that generates random sequences of transactions
to test invariants (properties that should always hold true).

Properties tested:

Fee calculation bounds
Access control enforcement
Amount constraints
No duplicate payments
Zero address validation
Integer overflow protection

github-actions · 2026-06-17T14:20:03Z

⚠️ Slither Security Analysis

Status: Issues Found

Findings Summary

Severity	Count	Status
✅ High	0	Pass
✅ Medium	0	Pass
🔵 Low	0	Info
ℹ️ Informational	0	Info

📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.

github-actions · 2026-06-17T14:22:11Z

❌ Echidna Fuzzing Results

Mode: ( test sequences)
Status: Property Violations Found

Property Test Results

Status	Count
✅ Passed	0
❌ Failed	0
Total	0
Pass Rate	0%

📄 Full report and corpus available in workflow artifacts.

ℹ️ About Echidna Fuzzing

Echidna is a property-based fuzzer that generates random sequences of transactions
to test invariants (properties that should always hold true).

Properties tested:

Fee calculation bounds
Access control enforcement
Amount constraints
No duplicate payments
Zero address validation
Integer overflow protection

github-actions · 2026-06-17T14:22:21Z

⚠️ Slither Security Analysis

Status: Issues Found

Findings Summary

Severity	Count	Status
✅ High	0	Pass
✅ Medium	0	Pass
🔵 Low	0	Info
ℹ️ Informational	0	Info

📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.

MantisClone

Changes to BatchConversionPayments.sol look correct 👍

Agreed that this does not need reentrancy guard. 👍

It will need to be a version 0.2.0 of the contract, with follow-on work to update the SDK.

There's no need to edit the BatchNoConversionPayments.sol contract. It's not deployed anywhere.

yomarion · 2026-06-18T09:15:10Z

Changes to BatchConversionPayments.sol look correct 👍

Agreed that this does not need reentrancy guard. 👍

It will need to be a version 0.2.0 of the contract, with follow-on work to update the SDK.

There's no need to edit the BatchNoConversionPayments.sol contract. It's not deployed anywhere.

Many thanks @MantisClone for the fast review

Yes, I agree with 0.2.0, see my branch name 😉

For BatchNoConversionPayments, I think you forget that:

contract BatchConversionPayments is BatchNoConversionPayments {

// ...

  function batchPayments(
    MetaDetail[] calldata metaDetails,
    address[][] calldata pathsToUSD,
    address feeAddress
  ) external payable {
  // ...
          batchFeeAmountUSD += _batchNativePayments(

MantisClone · 2026-06-18T12:30:47Z

Oh I see. it's inherited. Makes sense now 👍

github-actions Bot requested review from LeoSlrRf, MantisClone, alexandre-abrioux-rf and bassgeta June 17, 2026 13:51

yomarion force-pushed the smart-contracts/batch-conversion-0.2.0 branch from 557a081 to c69c779 Compare June 17, 2026 13:53

chore: medium vuln "Fee Address Is Using transfer"

d107428

yomarion force-pushed the smart-contracts/batch-conversion-0.2.0 branch from c69c779 to d107428 Compare June 17, 2026 13:56

greptile-apps Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/smart-contracts/src/contracts/BatchConversionPayments.sol

same fix for batch with no conversion

065ec1a

greptile-apps Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/smart-contracts/src/contracts/BatchNoConversionPayments.sol Outdated

Update packages/smart-contracts/src/contracts/BatchNoConversionPaymen…

c0616ed

…ts.sol Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

greptile-apps Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/smart-contracts/src/contracts/BatchNoConversionPayments.sol

MantisClone approved these changes Jun 17, 2026

View reviewed changes

Conversation

yomarion commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the changes

Uh oh!

greptile-apps Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

github-actions Bot commented Jun 17, 2026

❌ Echidna Fuzzing Results

Property Test Results

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Slither Security Analysis

Findings Summary

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Slither Security Analysis

Findings Summary

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Slither Security Analysis

Findings Summary

Uh oh!

github-actions Bot commented Jun 17, 2026

❌ Echidna Fuzzing Results

Property Test Results

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026

❌ Echidna Fuzzing Results

Property Test Results

Uh oh!

github-actions Bot commented Jun 17, 2026

✅ Echidna Fuzzing Results

Property Test Results

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Slither Security Analysis

Findings Summary

Uh oh!

github-actions Bot commented Jun 17, 2026

❌ Echidna Fuzzing Results

Property Test Results

Uh oh!

github-actions Bot commented Jun 17, 2026

⚠️ Slither Security Analysis

Findings Summary

Uh oh!

Uh oh!

MantisClone left a comment

Choose a reason for hiding this comment

Uh oh!

yomarion commented Jun 18, 2026

Uh oh!

MantisClone commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yomarion commented Jun 17, 2026 •

edited

Loading

greptile-apps Bot commented Jun 17, 2026 •

edited

Loading