Skip to content
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.

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

Project party can unilaterally change price payed at task completion #302

Closed
code423n4 opened this issue Aug 6, 2022 · 1 comment
Closed
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists valid

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L386
https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Project.sol#L330

Vulnerability details

Impact

If there has been more than a change in a task's cost through mulitple calls to changeOrder(), signatures previously passed can be replayed by one party to change the price payed for the task without consent of the other parties by frontrunning call to setComplete().

Proof of Concept

A task's cost can be changed multiple times during negotiations between builder/contractor and subcontractor by calling changeOrder(). Let's say it is changed two times before setComplete() is finally called and the subcontractor is payed. If for example the second change sets a cost which is smaller than the first change, then the subcontractor can replay the first change by frontrunning setComplete(), changing the order cost just before payment and managing to be payed more than expected.

Recommended Mitigation Steps

It is suggested to include a nonce into the signed data and comparing it with the current nonce (as done elsewhere in the code) to avoid signature replays.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 6, 2022
code423n4 added a commit that referenced this issue Aug 6, 2022
@parv3213
Copy link
Collaborator

Duplicate of #339

@parv3213 parv3213 marked this as a duplicate of #339 Aug 11, 2022
@itsmetechjay itsmetechjay added the duplicate This issue or pull request already exists label Aug 18, 2022
@jack-the-pug jack-the-pug added 3 (High Risk) Assets can be stolen/lost/compromised directly valid and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists valid
Projects
None yet
Development

No branches or pull requests

4 participants