CityCoins Audit – CoinFabrik Weblog

Introduction

CoinFabrik was requested to audit the contracts for the CityCoins venture. First we are going to
present a abstract of our discoveries after which we are going to present the small print of our
findings.

Scope

The contracts audited are from the https://github.com/citycoins/citycoin repository.
The audit relies on the commit 653056fe4f58b4eed5674009c55cedf0dc2dced3.
Fixes had been checked on 9dbc51e80e2653bd3d77175cd44c3e411ce2d7c9.

The audited contracts are:

● contracts/citycoin-vrf.clar: Incorporates the required capabilities to
generate an integer from the desired block’s VRF seed.
● contracts/citycoin-core-trait.clar: Trait definition for a core contract.
● contracts/citycoin-token-trait.clar: Trait definition for a token
contract.
● contracts/MiamiCoin/auth.clar: Administrative capabilities for the
approvers and metropolis pockets roles.
● contracts/MiamiCoin/token.clar: Token implementation.
● contracts/MiamiCoin/core-v1.clar: Incorporates mining, staking and reward
claiming capabilities.

Analyses

The next analyses had been carried out:
● Misuse of the completely different name strategies
● Integer overflow errors
● Division by zero errors
● Entrance working assaults
● Reentrancy assaults
● Misuse of block timestamps
● Softlock denial of service assaults
● Capabilities with extreme fuel price
● Lacking or misused operate qualifiers
● Needlessly advanced code and contract interactions
● Poor or nonexistent error dealing with
● Failure to make use of a withdrawal sample
● Inadequate validation of the enter parameters
● Incorrect dealing with of cryptographic signatures

Findings and Fixes

Severity Classification

Safety dangers are labeled as follows:
● Important: These are points that we handle to use. They compromise the
system severely. They should be fastened instantly.
● Medium: These are doubtlessly exploitable points. Regardless that we didn’t
handle to use them or their affect will not be clear, they may characterize a
safety danger within the close to future. We advise fixing them as quickly as potential.
● Minor: These points characterize issues which are comparatively small or troublesome
to reap the benefits of however could be exploited together with different points.
These sorts of points don’t block deployments in manufacturing environments.
They need to be taken into consideration and be fastened when potential.
● Enhancement: These sorts of findings don’t characterize a safety danger. They
are finest practices that we advise to implement.
This classification is summarized within the following desk:

Points Discovered by Severity

Important Severity Points

CR-01 Misplaced Miners Funds after Shut Down

Within the core contract, customers can decide to mine as many as the following 200 blocks. The
metropolis pockets or the approvers can improve the core contract by the auth contract
(auth.clar), stopping the mining within the earlier core. Subsequently, these miners who
dedicated for the blocks after the improve could have misplaced their dedicated tokens.

Advice

Implement a operate to refund miners which is known as by the shutdown operate
(shutdown-contract()). This operate ought to retailer the dedicated values in a
mapping. Lastly, miners would be capable to declare their tokens by calling a withdrawal
operate.

Standing

Acknowledged. Further analysis might be completed on this challenge. Within the meantime, the
growth group expressed that upgrades would solely be carried out when
completely essential, scheduled prematurely and well-communicated.

Medium Severity Points

ME-01 Insecure Authentication by tx-sender

International variable tx-sender returns the unique sender of the present transaction, or
if as-contract was referred to as to change the sending context, it returns that contract
principal. Utilizing this variable for authentication will not be safe. Actors within the system
may very well be targets of phishing. That is analogous to what occurs to tx.origin and
msg.sender in Solidity. There, the conference is to make use of msg.sender, which works
like contract-caller.
For example, an approver could be tricked to name a malicious contract which calls
auth.approve-job() in opposition to his will. Additionally, the malicious contract might name the
token switch operate (token.switch()) to ship the person’s tokens to itself.

Advice

Desire contract-caller to tx-sender for authentication. contract-caller
returns the caller of the present contract context.

Standing

Mitigated. Job approval stream requires both two-of-three multi-signature
transactions or three-of-five approvers’ votes.

Minor Severity Points

MI-01 Rounding Concern when Calculating Stacking Rewards

MI-01 Rounding Concern when Calculating Stacking Rewards

Within the core contract, stacking rewards are calculated by
get-entitled-stacking-reward(). The quantity of metropolis cash locked by the person is
multiplied by the full quantity of micro-stacks staked for a cycle, after which divided
by the full quantity of metropolis cash stacked for that cycle. Due to the info kind
imprecision, remainders might be accrued within the contract and won’t be used.

Advice
If there have been a substantial quantity of micro-stacks over time, the contract might
calculate this worth and use it. When all of the stackers have claimed their rewards,
the distinction between the claimed quantity and totalUstxThisCycle is the sum of
these remainders.

Standing
Acknowledged. The accrued worth will not be thought-about price sufficient to imagine
the runtime and implementation extra prices.

MI-02 Inactive Core Contract Can Activate Itself

All core contracts are saved within the auth contract with completely different states:
STATE_DEPLOYED, STATE_ACTIVE and STATE_INACTIVE. The deployed state is the
preliminary state. When it receives the required alerts, it transits to the lively state.
Lastly, whether it is upgraded, the earlier contract might be inactive.
auth.activate-core-contract() solely checks if the caller is within the core contract
mapping. If it had been a malicious contract which was changed by an improve, it
would name activate-core-contract() and activate itself.

Advice

Confirm the contract caller is a core registered with state STATE_DEPLOYED.

Standing

Mounted. The advice was applied.

MI-03 Malicious Inactive Core Contract Can Mint and Burn Tokens

The token contract (token.clar) solely validates if the contract caller is registered in
the auth contract. Nevertheless, contracts can’t be faraway from the auth contract.
Subsequently, if a malicious core had been added after which changed by the approvers, it
might nonetheless mint new tokens and burn another person’s tokens.

Advice

As a substitute of validating if the core contract is registered, the token contract ought to
examine if the core is lively by get-active-core-contract().

Standing

Acknowledged. This was a design selection. Core contracts want to have the ability to mint in
order to reward the miners who haven’t claimed their tokens earlier than the improve.
Additionally, the unintended inclusion of a malicious core contract is mitigated by the job
approval stream necessities (both two-of-three multi-signature transactions or
three-of-five approvers’ votes).

MI-04 Denial of Service within the Improve of a Malicious Core

upgrade-core-contract() and execute-upgrade-core-contract-job() are the
capabilities that set a brand new lively core contract. These capabilities name
shutdown-contract() from the present lively core to disable it. Nevertheless, if the
present core is a malicious contract, it’d revert to cease the improve, making it
inconceivable to alter the core.

Advice

As a substitute of calling the core contract to change the variables, these variables may very well be
positioned within the auth contract and be checked by the core by exterior calls.
Subsequently, the improve doesn’t rely upon unknown exterior logic.

Standing

Mitigated. The unintended inclusion of a malicious core contract is mitigated by the
job approval stream necessities (both two-of-three multi-signature transactions or
three-of-five approvers’ votes).

Enhancements

EN-01 Approvers Have to Approve his Personal Jobs

Within the auth contract (auth.clar), approvers can create a job. For a job to be
executed, it requires a minimal quantity of approvals. Now, if an approver creates a
job by create-job(), he can name approve-job() to approve his personal job.

Advice

The job creation operate must also add the creator approval. Then, the job creator
doesn’t must make one other transaction to approve his personal job.

Standing

Not fastened.

EN-02 Inactive Core Contract Will be Goal of an Improve

Within the auth contract (auth.clar), upgrade-core-contract() and
execute-upgrade-core-contract-job() doesn’t examine if the brand new core contract
was already registered earlier than. Subsequently, an inactive core could be handed as an
argument after which be activated. This may lead to a poor expertise as a result of
that core was shut down when it was changed and many of the capabilities are
disabled.

Advice

Validate the brand new contract will not be already registered within the auth contract.

Standing

Mounted.

Different Issues

● The burn operate (token.burn()) permits a core contract to burn customers’
tokens with out their consent. The audited core contract doesn’t implement
any operate which calls token.burn(), however a brand new core contract could be
included and name that operate.
The brand new commit modified the burn() operate, permitting solely the homeowners to
burn their very own tokens.

Conclusion

We discovered the contracts to be easy and simple and have an enough
quantity of documentation. We discovered a vital challenge, a medium challenge and several other
minor points and enhancements.
A difficulty was fastened, two had been mitigated and three had been acknowledged.

Disclaimer: This audit report will not be a safety guarantee, funding recommendation, or an
approval of the CityCoins venture since CoinFabrik has not reviewed its platform.
Furthermore, it doesn’t present a sensible contract code faultlessness assure.