EthereumCommonwealth / Auditing

Ethereum Commonwealth Security Department conducted over 400 security audits since 2018. Not even a single contract that we audited was hacked. You can access our audit reports in the ISSUES of this repo. We are accepting new audit requests.
https://audits.callisto.network/
GNU General Public License v3.0
131 stars 34 forks source link

Dirham Token #475

Closed PaulBartez closed 3 years ago

PaulBartez commented 3 years ago

Audit request

Dirham is a fiat collateralized stablecoin backed by AED. It is the native to Dirham crypto where bonds are introduced to blockchain for the first time ever. Dirham holders earn 4% ineterest every week. Paying interest done by calling the rebase function in smart contract.

Source code

https://github.com/DirhamCrypto/DirhamToken

Disclosure policy

Standard disclosure policy.

Contact information (optional)

paulbartez@dirhamcrypto.io https://dirhamcrypto.io https://twitter.com/DirhamCrypto

Platform

ETH

yuriy77k commented 3 years ago

The audit fee is 657 USDT. You may send USDT (ERC20 or BEP20) to: 0xb9662e592f2f0412be62f0833ca463a9b1aabebb or USDT (TRC20) to: TBzUKbek9AYVBwf91ykh3KY4Ushk95SCiB

The estimated auditing time - 5 days after payment.

MrCrambo commented 3 years ago

Auditing time is 2 days.

yuriy77k commented 3 years ago

@MrCrambo assigned

yuriy77k commented 3 years ago

Dirham Token Security Audit Report

1. Summary

@openzeppelin/contracts-ethereum-package Token smart contract security audit report performed by Callisto Security Audit Department

Dirham is a fiat collateralized stablecoin backed by AED. It is the native to Dirham crypto where bonds are introduced to blockchain for the first time ever. Dirham holders earn 4% interest every week. Paying interest done by calling the rebase function in smart contract.

2. In scope

Commit e4a9dc34f9020e7733a289b9b9b4a3d74daee1a1

2.1. Excluded

Openzeppelin library:

3. Findings

In total, 5 issues were reported including:

No critical security issues were found.

3.1. Known vulnerabilities of ERC-20 token

Severity: low

Description

  1. Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here.

Recommendation

Add the following code to the transfer(_to address, ...) function:

require( _to != address(this) );
  1. ERC20 is a widely used standard across the Ethereum ecosystem. It is reasonable to assume that ERC20 tokens could be "accidentally" deposited to this contract even though it is not intentional.

Every user on the entire Ethereum ecosystem can send ERC20 tokens to this contract and he will have no ability to extract it back unless there is a special "ERC20-rescue" function implemented in your contract. It is advised to implement this function.

Example: here is BAT contract address. As you can see the contract itself holds $497,000 worth of different ERC20 tokens - all these tokens are permanently "stuck" inside the contract and therefore uselessly lost.

Recommendation

A simple "ERC20-rescue" function can solve the problem.

interface IERC20 {
  function transfer(address _to, unit _amount);
}

function rescueERC20(address _token, uint256 _amount) external onlyOwner {
    IERC20(_token).transfer(owner(), _amount);
  }

3.2. Owner privileges

Severity: owner privileges

Description

  1. Owner can emit fake transfer events, this could be risky if exchanges will work with this token and evaluate transfers using Transfer event.
  2. User with MINTER_ROLE can mint any amount of tokens.
  3. User with REBASER_ROLE can set rebase factor to any value without restriction and can call function rebase() as often as he wants. In this case the smart contract can't guarantee that Dirham holders earn 4% interest every week as was said in description.
  4. Owner has DEFAULT_ADMIN_ROLE and can set/remove MINTER_ROLE and REBASER_ROLE to any address.

4. Conclusion

The audited smart contract can be deployed. Only low severity issues were found during the audit.

5. Revealing audit reports

https://gist.github.com/MrCrambo/dd3f22539e06d502b8b678b7cc705112

PaulBartez commented 3 years ago

Thanks for your audit report. I think it was a mistake when I write in the audit request description that "Dirham holders earn 4% interest every week.". The true one is that "Dirham holders earn 4% interest every year." and also there is not any insurance that holders can earn exactly 4% interest at end of the year. It is a estimation.

Another issue we had is that what we should do after correcting problems you mentioned in the report.

yuriy77k commented 3 years ago

@PaulBartez After fixing issues, you may order a re-audit with a 50% discount. You may send 328 USDT (ERC20 or BEP20) to: 0xb9662e592f2f0412be62f0833ca463a9b1aabebb or (TRC20) to: TBzUKbek9AYVBwf91ykh3KY4Ushk95SCiB

After payment, write here the link to the updated contract.

PaulBartez commented 3 years ago

Based on what you mentioned in section 3.1 I think there is no need for a huge change in our code so isn't it possible to agree on a lower amount for re-auditing?

yuriy77k commented 3 years ago

We discussed your case, and decide to re-audit your contract for free (as a bonus that you want to fix low severity issues). Many developers ignore such issues.

PaulBartez commented 3 years ago

Here is the link for the updated contract: https://github.com/DirhamCrypto/DirhamToken Please note that the amount of interest we pay is a matter of our business plan and it's not related to our smart contract. By the way, the owner of our contract would be gnosis and the gnosis will have the owner privileges.

yuriy77k commented 3 years ago

Dirham Token Security Audit Report v.2

1. Summary

@openzeppelin/contracts-ethereum-package Token smart contract security audit report performed by Callisto Security Audit Department

Dirham is a fiat collateralized stablecoin backed by AED. It is native to Dirham crypto where bonds are introduced to blockchain for the first time ever. Dirham holders may earn interest. Paying interest done by calling the rebase function in the smart contract.

2. In scope

Commit d67a5f947ae0eaa8000021e8493181eb9475b1ad

2.1. Excluded

Openzeppelin library:

3.1. Owner privileges

Severity: owner privileges

Description

Smart contract owners use the Gnosis multisig wallet that increases private key security. Owner may:

  1. Owner has DEFAULT_ADMIN_ROLE and can set/remove MINTER_ROLE and REBASER_ROLE to any address.
  2. User with MINTER_ROLE can mint any amount of tokens.
  3. User with REBASER_ROLE can set rebase factor to any value without restriction and can call function rebase() at any time.

These owners' privileges are required for the functionality of stablecoin.

4. Conclusion

The audited smart contract can be deployed. The issues that were pointed in the previous audit report were fixed.

5. Revealing previous audit reports

https://gist.github.com/yuriy77k/36b16c93cd3c3a3bdf52cc79005bce07

PaulBartez commented 3 years ago

Hi there, Can you please create a new gist page for the new audit report?

yuriy77k commented 3 years ago

@PaulBartez https://gist.github.com/yuriy77k/3cb2af0e4fd271cdf48297a174f86b43

PaulBartez commented 3 years ago

Should we do anything to see our audit report on https://callisto.network/security-audits/?

yuriy77k commented 3 years ago

@PaulBartez Blog post: https://callisto.network/dirham-token-soy-security-audit/

Twitter: https://twitter.com/Callisto_Audits/status/1379526923882270720

Twitter FR: https://twitter.com/CallistoNetFr/status/1379526915632066562

Twitter RU: https://twitter.com/CallistoNetRu/status/1379526907197263881