A malicious actor may prevent a counterpart from submitting a valid rating

[jakub-wojciechowski]

Summary

A malicious actor may exploit a vulnerability in the contract to a block a counterpart from submitting a valid rating. Such action will harm the reputation of a counterpart.

Steps to Reproduce (for bugs)

Calling the colony.submitTaskWorkRating twice, with the same taskId, role and and an empty rating secret:

const WORKER_RATING_SECRET = web3.utils.fromAscii("");
await colony.submitTaskWorkRating(taskId, WORKER_ROLE, WORKER_RATING_SECRET, { from: evaluator });
await colony.submitTaskWorkRating(taskId, WORKER_ROLE, WORKER_RATING_SECRET, { from: evaluator });

Expected Behavior

A submission on an empty string should be rejected as it's a special value that indicates a lack of rating which is validated in the ratingSecretDoesNotExist modifier:

require(taskWorkRatings[_id].secret[_role] == "", "colony-task-rating-secret-already-exists");

Current Behaviour

Although the submission regards the same role the ratingSecrets.count counter is increased twice. Any further submissions, for a different role, are now going to break the taskWorkRatingCommitOpen by failing the check:

require(ratingSecrets.count < 2, "colony-task-rating-all-secrets-submitted");

Therefore the counterpart is blocked from submitting a valid rating and it will receive a reputation penalty for which it shouldn't be blamed at all.

Possible Solution

I suggest validating the _ratingSecret input argument in the ColonyTask to check if the value is not empty.

[elenadimitrova]

Thanks for the submission @jakub-wojciechowski . We acknowledge it's an issue and we'll get back to you later this week with an accurate determination of risk level. The issue here will be fixed and closed before that but it is still considered.

[jakub-wojciechowski]

Thanks @elenadimitrova, there is absolutely no problem with the delay. I'm enjoying reading the code, it's one of the most interesting projects I've seen in the space!

[elenadimitrova]

Thanks for the submission @jakub-wojciechowski !
We've actually found a further exploit of this by a malicious task worker. It consists of an empty rating secret being accepted first, then a valid rating of the manager accepted second. Both submitted by the worker who can then continue to reveal their valid rating and after 5 days finalize the task.
This effectively means the evaluator gets unfairly penalised reputation-wise for not rating on time while the worker gets the maximum rating and reputation. All task roles can claim their payouts.

Overall the likelihood is towards the low end of "Medium" in owasp terms as this is only exploitable by poorly performing task worker or evaluator with some level of technical knowledge (submitting an empty rating won't be possible through the dApp). The impact is "Low" as this only affects the worker and evaluator not earning the correct reputation and worker potentially getting paid when they shouldn't have been. Even though this can potentially affect multiple tasks across multiple colonies, the 5 day buffer period lowers the impact considerably.

Likelihood: Medium
Impact: Low
OWASP: Low, but would like to reward you in the mid-range of the pay bracket at $1,200