Submitted by 0xA5DF, also found by Bahurum, bin2chen, byndooa, cryptphi, hansfriese, horsefacts, kaden, Lambda, neumo, panprog, rokinot, scaraven, and sseefried
Project.sol#L386-L490
Project.sol#L330-L359
Tasks.sol#L153-L164
This attack path is the results of signatures reusing in 2 functions - changeOrder() and setComplete(), and a missing modifier at Tasks.unApprove() library function.
Current or previous subcontractor of a task can drain the project out of its funds by running setComplete() multiple times.
This can be exploited in 3 scenarios:
After setComplete() ran once by the legitimate users (i.e. signed by contractor, SC and builder), the attackers can now run it multiple times:
This can also be used by external users (you dont need to be builder/GC/SC in order to run changeOrder()) to troll the system (This still requires the task to be changed at least twice, otherwise re-running changeOrder() with the same data would have no effect).
Since the tests depend on each other, the PoC tests were created by adding them to the file test/utils/projectTests.ts, after the function it('should be able to complete a task' (Line 1143).
In the first test - a subcontractor is changed and then changed back.
In the second scenario a price is changed to the new price (that is higher than the total available funds, and therefore is unapproved) and then back to its original price (it can actually be any price that is not higher than the available funds).
In both cases Im demonstrating how the project can be drained out of fund.
Note: see wardens original submission for full proof of concept.
Hardhat
zgorizzo69 (Rigor) confirmed and commented:
