I have used the payment split contract as part of a bigger project which is currently undergoing audit.
The auditors have picked up the following in the Payment Split Contract Code which I wanted to verify with the Zeppelin community before I make changes.
Critical-2. Payment is calculated wrong in case a new payee is added. (Unresolved)
In case, a new payee is added, pending payment for other users will be calculated wrong in case they have released tokens before.
User has a share, equal to 10. Total shares is 100.
User1 calls release(user1Address).
totalReceived = 100 ETH.
payment = 100 * 10 / 100 - 0.
_released(user1Address) = 10
_totalReleased = 10.
After that, another user is added with a share, equal to 10. total shares is 110.
User1 calls release again.
totalReceived = 105 ETH. (Additional ETH was transferred to contract).
payment = 105 * 10 / 110 - 10 = 9.54 - 10. An underflow is occured, preventing user from claiming. Future payment might also be corrupted.
Recalculate shares for users, so that no underflow occur and payments are paid as intended
To give some context the contract should take in ETH and the split it between different 'share holders' depending on how many shares they own. It is one of the Open Zeppelin Finance contracts.
To claim their money a user calls the release function, at which point the amount they are owed is released to them based on what they have had before and their shareholding.
thanks for linking the contract, didnt know you were talking about a public one provided by OpenZeppelin.
have you modified the contract in your own project? the _addPayee method is private and only called in the constructor, so I am not sure how you are adding another user after the construction
Yeah thanks guys, I realised the same, I have modified it to allow adding additional payees which is causing the issue.
In the end I have added functionality to allowing adding new payees, but before any releases can happen the contract needs to be locked from adding new Payees. This allows me to deploy without the complete list of payees but also avoid the underflow error.