Skip to content
This repository has been archived by the owner on Dec 22, 2024. It is now read-only.

PNS - Loss Calculation Bug in Request Execution #175

Closed
sherlock-admin2 opened this issue Jun 20, 2024 · 0 comments
Closed

PNS - Loss Calculation Bug in Request Execution #175

sherlock-admin2 opened this issue Jun 20, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Jun 20, 2024

PNS

Medium

Loss Calculation Bug in Request Execution

Summary

The protocol incorrectly calculates the loss incurred during the execution of a request. As a result, the protocol will not be able to accurately track its losses.

Vulnerability Detail

he GasProcess:processExecutionFee() function is used to calculate and verify how much the user has paid for executing a request. If the paid amount is less than the actual gas cost, the protocol records a loss. However, due to the incorrect order of operations, the loss is always zeroed out.

File: contracts/process/GasProcess.sol
22:         if (executionFee > cache.userExecutionFee) {
23:             executionFee = cache.userExecutionFee;
24:             lossFee = executionFee - cache.userExecutionFee; //@audit always 0
25:         } else {

The current logic sets executionFee to cache.userExecutionFee before calculating lossFee, which results in lossFee always being zero.

Impact

The protocol will not correctly record the loss, leading to inaccurate financial tracking and potential mismanagement of funds.

Code Snippet

Tool used

Manual Review

Recommendation

hange the order of operations to correctly calculate the loss before updating the executionFee.

        if (executionFee > cache.userExecutionFee) {
+           lossFee = executionFee - cache.userExecutionFee;
            executionFee = cache.userExecutionFee;
-           lossFee = executionFee - cache.userExecutionFee;
        } else {

This adjustment ensures that lossFee is calculated based on the original executionFee before it is modified to match cache.userExecutionFee.

Duplicate of #108

@sherlock-admin3 sherlock-admin3 added Won't Fix The sponsor confirmed this issue will not be fixed and removed Won't Fix The sponsor confirmed this issue will not be fixed labels Jun 21, 2024
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jun 27, 2024
@sherlock-admin3 sherlock-admin3 added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed labels Jul 3, 2024
@sherlock-admin2 sherlock-admin2 changed the title Scrawny Bone Goblin - Loss Calculation Bug in Request Execution PNS - Loss Calculation Bug in Request Execution Jul 3, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

2 participants