Flow

Sablier
FoundryDeFi
20,000 USDC
View results
Submission Details
Severity: medium
Invalid

Lack Of Approval in `SablierFlow`::adjustRatePerSecond

Summary

The SablierFlow contract allows the sender of a stream to initiate a rate adjustment without requiring the approval of the recipient or admin. While the contract has been updated to include a mechanism for initiating and approving rate adjustments, there are concerns about the potential for malicious behaviour, such as adjusting the rate to an undesirable value without the knowledge or consent of the other party involved in the transaction. This could result in financial loss or exploitation of the recipient.

Vulnerability Details

ratePerSecond Adjustment Without Approval: The sender can initiate a rate adjustment using `adjustRatePerSecond` without any requirement for approval. The recipient has no control over the initiation of this adjustment.

Vulnerability can be found here:https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/SablierFlow.sol#L542-L561

@> function _adjustRatePerSecond(uint256 streamId, UD21x18 newRatePerSecond) internal {
// Check: the new rate per second is different from the current rate per second.
if (newRatePerSecond.unwrap() == _streams[streamId].ratePerSecond.unwrap()) {
revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
}
uint256 ongoingDebtScaled = _ongoingDebtScaledOf(streamId);
// Update the snapshot debt only if the stream has ongoing debt.
if (ongoingDebtScaled > 0) {
// Effect: update the snapshot debt.
_streams[streamId].snapshotDebtScaled += ongoingDebtScaled;
}
// Effect: update the snapshot time.
_streams[streamId].snapshotTime = uint40(block.timestamp);
// Effect: set the new rate per second.
_streams[streamId].ratePerSecond = newRatePerSecond;
}

Impact

The ability for the sender to unilaterally change the rate per second poses several risks, including:

  1. Financial Loss: The recipient may find themselves in a situation where their income from the stream is dramatically reduced or eliminated entirely if the sender sets the rate to an undesirable value.

  2. Trust Issues: The nature of the agreement between the sender and recipient may be compromised, eroding trust in the SablierFlow contract as a fair payment mechanism.

  3. Exploitability: Malicious actors could exploit this vulnerability by creating contracts that manipulate the rate adjustment process, allowing for significant financial manipulation.

Tools Used

Manual review

Recommendations

To improve the security of the SablierFlow contract and mitigate the identified vulnerabilities, it is recommended to modify the _adjustRatePerSecond function to require approval from either the recipient or an admin before the sender can initiate any rate adjustments. This modification will ensure that all parties involved consent to any changes made. Below is the updated code along with tests to confirm that initiateRateAdjustment called by the sender and approveRateAdjustment work effectively together to enhance the protocol's security.

  1. Adjusted code for SablierFlowcontract.

    _streamRecipients;mapping was added for mapping recipient's approvals to streamId, this can also be done by adding the recipient to the Flow::Streamstreet in DataTypes

event RateAdjustmentInitiated(uint256 indexed streamId, UD21x18 newRatePerSecond);
event RateAdjustmentApproved(uint256 indexed streamId, UD21x18 newRatePerSecond);
//Struct to hold pending rate adjustment
struct RateAdjustmentRequest {
UD21x18 newRatePerSecond;
bool isPending;
}
mapping(uint256 => RateAdjustmentRequest) private _pendingRateAdjustments;
mapping(uint256 => address) private _streamRecipients;
//address public recipient;
//Modifier to restrict access to the stream recipient or admin only
modifier onlyRecipientOrAdmin(uint256 streamId) {
address recipient = _streamRecipients[streamId];
require(msg.sender == recipient || msg.sender == admin, "Not authorized: must be recipient or admin");
_;
}
// Add a public view function to retrieve pending rate adjustment details for a given stream ID
function getPendingRateAdjustment(uint256 streamId)
external
view
returns (UD21x18 newRatePerSecond, bool isPending)
{
// Retrieve the pending rate adjustment request for the specified stream ID
RateAdjustmentRequest memory request = _pendingRateAdjustments[streamId];
// Return the new rate per second and the pending status
return (request.newRatePerSecond, request.isPending);
}
//Function to initiate rate adjustment by the sender
function initiateRateAdjustment(uint256 streamId, UD21x18 newRatePerSecond) external {
//Ensure the new rate per second is different from the current rate
if(newRatePerSecond.unwrap() == _streams[streamId].ratePerSecond.unwrap()) {
revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
}
//Store the requested rate in the pending adjustments mapping
_pendingRateAdjustments[streamId] = RateAdjustmentRequest({
newRatePerSecond: newRatePerSecond,
isPending: true
});
// Emit an event to notify the recipient or admin to approve the rate adjustment
emit RateAdjustmentInitiated(streamId, newRatePerSecond);
}
// Function to approve and finalize rate adjustment by the recipient or admin
function approveRateAdjustment(uint256 streamId) external onlyRecipientOrAdmin(streamId) {
RateAdjustmentRequest storage request = _pendingRateAdjustments[streamId];
//Ensure there's a pending request
require(request.isPending, "No pending rate adjustments");
uint256 ongoingDebtScaled = _ongoingDebtScaledOf(streamId);
//Update snapshot debt if the stream has ongoing debt
if (ongoingDebtScaled > 0) {
_streams[streamId].snapshotDebtScaled += ongoingDebtScaled;
}
//Update snapshot time
_streams[streamId].snapshotTime = uint40(block.timestamp);
//Set the new rate per second to the approved value
_streams[streamId].ratePerSecond = request.newRatePerSecond;
//clear the pending request
request.isPending = false;
// Emit an event indicating approval and completion
emit RateAdjustmentApproved(streamId, request.newRatePerSecond);
}
  1. Test code for the above functions

contract SablierFlowApprovalForAdjustmentOfRatePerSecondTest is Test {
SablierFlow public flow;
ERC20Mock token;
address public sender = address(0x1);
address public recipient = address(0x2);
address public initialAdmin = address(0x3);
UD21x18 initialRatePerSecond = UD21x18.wrap(1 ether);
UD21x18 newRatePerSecond = UD21x18.wrap(2 ether);
address public admin = address(0x3);
//uint256 public streamId = 1;
function setUp() public {
flow = new SablierFlow(initialAdmin, IFlowNFTDescriptor(address(0)));
token = new ERC20Mock("TestToken", "TTK", 18);
token.mint(sender, 100 ether);
vm.prank(sender);
token.approve(address(flow), 100 ether);
}
function testInitiateAndApproverateAdjustment() public {
//1. Create a stream
uint256 streamId = flow.create(sender, recipient, initialRatePerSecond, IERC20(address(token)), true);
//Assert stream Created with initial rate
Flow.Stream memory stream = flow.getStream(streamId);
assertEq(stream.ratePerSecond.unwrap(), initialRatePerSecond.unwrap());
//2. Initiate a rate adjustment as the sender
flow.initiateRateAdjustment(streamId, newRatePerSecond);
//Check that rate adjustment is pending
(, bool isPending) = flow.getPendingRateAdjustment(streamId);
assertTrue(isPending, "Rate adjustment should be pending");
//3. Approve the rate adjustment as the recipient
vm.stopPrank();
vm.startPrank(admin);
flow.approveRateAdjustment(streamId);
// 4. Check final rate and state
stream = flow.getStream(streamId);
assertEq(stream.ratePerSecond.unwrap(), newRatePerSecond.unwrap(), "Rate per second should match the new rate");
//confirm no pending adjustment remain for the stream
(, isPending) = flow.getPendingRateAdjustment(streamId);
assertFalse(isPending, "No pending rate adjustment should remain after approval");
}
}
  1. Adjustments to the createfunction because of the mapping created to map the recipient's approvals to streamId

function create(
address sender,
address recipient,
UD21x18 ratePerSecond,
IERC20 token,
bool transferable
)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Checks, Effects, and Interactions: create the stream.
streamId = _create(sender, recipient, ratePerSecond, token, transferable);
+ _streamRecipients[streamId] = recipient;
+ return streamId;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.