Do security audit of the smart contract

[vporton]

Do a security audit of this Solidity smart contract:

https://github.com/vporton/courts/blob/master/contracts/RewardCourts.sol

Apply only if you have security audit experience.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 0.799 ETH (90.03 USD @ $112.68/ETH) attached to it as part of the vporton fund.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $62,543.07 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 3 weeks, 3 days from now.
Please review their action plans below:

1) ridesolo has been approved to start work.

hello, I'm an expert solidity auditor and I'm interested to audit your contract, you can check all my reports in my gist http://gist.github.com/ridesolo/. I have audited hundreds of projects the lasts two years.
Also I like to take my time to do a good work so hope you are not in a hurry.

Learn more on the Gitcoin Issue Details page.

[gitcoinbot]

@RideSolo Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• reminder (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[RideSolo]

Working on it.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 0.799 ETH (93.56 USD @ $117.09/ETH) has been submitted by:

1. @ridesolo

@vporton please take a look at the submitted work:

• PR by @RideSolo

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $78,525.15 more funded OSS Work available on the Gitcoin Issue Explorer

[vporton]

My response to your security audit: https://gist.github.com/vporton/e39a060986d02e1889aea0b50aa4356c - please respond back.

[RideSolo]

@vporton

i Index

For i index you are right when you say "i represent the position in the _trustees list which are courts to untrust" but the error is here:

trustedCourtsList[_truster][i] = trustedCourtsList[_truster][--max]; 

you are setting the last element at index i, but i does not represent the index of the court to be removed, it represent its position in _trustees array (not the position in trustedCourtsList array).

createIntercourtToken

As I commented "No owner is constrained to follow the id returned by createIntercourtToken", meaning that you didn't implement any mechanism to block owners from using the same intercourt token id. The function does return a unique id at each call, but any owner can use the same id.

Trusted courts

This is not a programing error, If you intended it to be like this then it is all right, I wanted to highlight the risks taken when implementing such logic.

Please note that as a general remark, I always try to stay neutral when I conduct an audit, I try to highlights the risks for both project team and users.

[vporton]

Why have you removed this?

        if (_interfaceId == INTERFACE_SIGNATURE_ERC165 ||
             _interfaceId == INTERFACE_SIGNATURE_ERC1155) {
          return true;
        }

You proposed to add checks for methods, but removed the check for these two interfaces.

I do not understand.

[vporton]

Could you provide the full code for setSupportedInterfaces()/supportsInterface()?

Note that the last version of the contract added two more public variables, which are also to be counted.

[vporton]

I do not understand why you suggest to rewrite

    function supportsInterface(bytes4 _interfaceId)
    public
    view
    returns (bool) {
        if (_interfaceId == INTERFACE_SIGNATURE_ERC165 ||
            _interfaceId == INTERFACE_SIGNATURE_ERC1155) {
          return true;
        }

        return false;
    }

in another way. You tell something about gas, but it's obvious that this function uses very little gas.

[RideSolo]

@vporton sure like this it is too low, however I saw that you commented out part of the value here, I set this issue to low, since If you wanted to list all the functions following ERC-165, the required gas to execute the function will be too high (probably higher than 10k or 30k suggested by the standards).

This is an audit, the recommendation done was to implement a more efficient way to list the functions, as commented " list all public function here, following the previous example" you can simply add the remaining functions by following the same example. The proposed implementation is more gas efficient since you can list as many function selectors as you want.

Also there was an error, since you set another function selector to true that must not be implemented, this is why I suggested a new way to implement it. as you can see the original code below.

    function supportsInterface(bytes4 _interfaceId)
    public
    view
    returns (bool) {
        if (_interfaceId == INTERFACE_SIGNATURE_ERC165 ||
            _interfaceId == INTERFACE_SIGNATURE_ERC1155 ||
            _interfaceId == INTERFACE_SIGNATURE_URI) {
          return true;
        }

        return false;
    }

Please note that it was just a suggestion and you can decide to ignore it. the main point was to point out the issue in case of setting more function selectors.

[vporton]

Why should I add "all functions"?

INTERFACE_SIGNATURE_ERC165 and INTERFACE_SIGNATURE_ERC1155 are already checked. I am confused.

[RideSolo]

I agree, I pointed this issue out because I saw you comments here I figured out that you wanted to implement all functions and using condition while accessing all global variables won't be optimal. also you should note that it is a low severity issue and it does not represent any risk .

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

The funding of 0.799 ETH (93.56 USD @ $117.09/ETH) attached to this issue has been approved & issued to @RideSolo.

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $78,284.97 more funded OSS Work available on the Gitcoin Issue Explorer