MEDIA Protocol Contracts Security Audit

In this report we consider the security of the MEDIA Protocol project. Our task is to find and describe security issues in the smart contracts of the platform.


Procedure

In our audit, we consider the following crucial features of the smart contract code:

  1. Whether the code is secure.
  2. Whether the code corresponds to the documentation (including whitepaper).
  3. Whether the code meets ​best​ ​practices​ ​in​ ​efficient​ ​use​ ​of​ ​gas,​ ​code readability, etc.

We perform our audit according to the following procedure:

Automated analysis

  • we scan project’s smart contracts with our own Solidity static code analyzer SmartCheck
  • we scan project’s smart contracts with several publicly available automated Solidity analysis tools such as Remix, Oyente, and Solhint
  • we manually verify (reject or confirm) all the issues found by tools

Manual audit

  • we manually analyze smart contracts for security vulnerabilities
  • we check smart contracts logic and compare it with the one described in the whitepaper
  • we check ERC20 compliance
  • we run tests and check code coverage

Report

  • we reflect all the gathered information in the report

Disclaimer

The audit does not give any warranties on the security of the code. One audit can not be considered enough. We always recommend proceeding with several independent audits and a public bug bounty program to ensure the security of smart contracts. Besides, security audit is not an investment advice.



Project overview

In our analysis we consider MEDIA Protocol whitepaper (MEDIA Protocol Technical White Paper MASTER.docx (1).pdf, sha1sum 3d132737f415a1b0e8a49a9fc09b785560976047) and smart contracts code (media-token-master.zip, sha1sum 96b45bb9a0e9900754396abb285ba040867d15a4, contains git-repository, version on commit ce9e4aafb867b6ed6d395aa8758cc9fd2c9c22ef)).

The latest version of the code

We have performed the check of the fixed vulnerabilities in the latest version of code —

Git repository, version on commit efdcb7d, or MediaProtocol-master.zip, sha1sum d297f8da2f4efcac11133578fdba41e5a232d74e.


Project architecture

For the audit, we have been provided with the following set of files:


Code logic

The basis of the smart contract architecture is the upgradeable smart contract as implemented in Upgradeable.sol. The idea of upgradeable smart contract is that the contract is separated in two parts: UpgradeableDispatcher and UpgradeableImplementer.

UpgradeableDispatcher

  • serves as a front-end proxy and forwards (delegates) all messages to the implementer of UpgradeableImplementer contract
  • all state variables are stored in the dispatcher contract
  • allows implementers to have access to the dispatcher’s state variable through the delegatecall function
  • stores the address of the current implementer in _implementer state variable
  • current implementer may be replaced at any moment by calling the replaceImplementation function
  • after a new implementer is installed, its initialize method is invoked via delegatecall that means that the method gets all state variables of the dispatcher contract rather than the implementer contract
  • only the owner of the dispatcher smart contract may replace the implementation
  • is created with no owner (_owner == 0) set
  • the first call to setOwner function sets the owner
  • after the call the owner resetting is not possible.

MediaToken

  • ERC20 compatible token with the following parameters:
    Name: MediaToken
    Symbol: MEDIA
    Decimals: 18
  • supports recurrent payments
  • the owner may approve spending of some amount of tokens per specified interval
  • interval is measured in blocks
  • approveRecurrent function creates or updates the approval for a specified address
  • canSpend function checks if the given spender may spend the given value of tokens on behalf of the from address.

MediaManager

  • is the managing contract
  • is created simultaneously with MediaToken
  • has the same owner as MediaToken
  • may perform token transfers on behalf of any user
  • a user may disable such transfers by calling disableManager function
  • once disabled there is no way to re-enable MediaManager services.

The central smart contract is PromotionsImplementation. It inherits all implementation contracts:

  • InteractionsCounterImplementation
  • SubscriptionManagerImplementation
  • ContentPurchaseTrackerImplementation.

The proxy contract PromotionsDispatcher inherits:

  • InteractionsCounterDispatcher
  • SubscriptionManagerDispatcher
  • ContentPurchaseTrackerDispatcher.

The constructor of PromotionsDispatcher sets the owner and the implementation for the dispatcher.

The following contracts are not intended for deployment but are inherited by other smart contracts:

  • ContentPurchaseTracker
  • ContentPurchaseTrackerDispatcher
  • ContentPurchaseTrackerImplementation (that is whole ContentPurchaseTracker.sol)
  • Delegatable
  • DelegatableDispatcher
  • DelegatableImplementation (Delegatable.sol)
  • InteractionsCounterStorage
  • InteractionsCounter
  • InteractionsCounterDispatcher
  • InteractionsCounterImplementation (InteractionsCounter.sol)
  • MediaTokenUser (MediaTokenUser.sol)
  • Promotion (Promotion.sol)
  • Promotions, PromotionsStorage
  • StandardTokenWithRecurrency
  • SubscriptionManager
  • SubscriptionManagerStorage
  • SubscriptionManagerDispatcher
  • SubscriptionManagerImplementation (SubscriptionManager.sol).

Deployment

The deployment is performed by script 2_deploy_contracts.js.

MediaToken smart contract is deployed with the initial balance of 1,000,000 MediaTokens. MediaToken deploys MediaManager smart contract.

Then promotionLibrary and subscriptionManagerLibrary are deployed.

Then PromotionsImplementation is deployed followed by PromotionsDispatcher.

Finally SimpleSubscriptionDefinition is deployed.

IdentityVerification is not deployed by the script.


Automated analysis

We used several publicly available automated Solidity analysis tools.

Here are the combined results of SmartCheck, Solhint, and Remix. Oyente has found no issues.

All the issues found by tools were manually checked (rejected or confirmed).

Cases when these issues lead to actual bugs or vulnerabilities are described in the next section.


Manual analysis

The contracts were completely manually analyzed, their logic was checked and compared with the one described in the documentation. Besides, the results of the automated analysis were manually verified. All confirmed issues are described below.


Critical issues

Critical issues seriously endanger smart contracts security. We highly recommend fixing them.

Unchecked Call

In Upgradeable.sol, line 33, delegatecall is used without checking return value:

_implementation.delegatecall(bytes4(keccak256(“initialize()”)));

Expect calls to external contract to fail. When sending ether, check for the return value and handle errors. We recommend using transfer for ether transfers.

The issue has been fixed and is not present in the latest version of the code.

Medium severity issues

Medium issues can influence smart contracts operation in current implementation. We highly recommend addressing them.

Obsolete code

The dispatcher smart contract delegates actual function calls to the implementer using the default function handler feature of Solidity (Upgradeable.sol, line 59).

The default function handler obtains the signature of the function being called. The expected length of the return value of the function is expected to be stored in the _returnSizes map. This implies that prior to any call to the dispatcher is possible, the implementer must setup _returnSizes map for all functions that may be called via the dispatcher. This operation should be performed by initialize() function, which is called by the dispatcher when the implementation is installed by replaceImplementation function. Note, however, that the current version of EVM supports RETURNDATASIZE and RETURNDATACOPY instructions. These instructions make return value size bookkeeping in _returnSizes map unnecessary.

We recommend upgrading the dispatcher contract source code.

The issue has been fixed and is not present in the latest version of the code.

Code logic error

  • canSpend function logic error

If the current interval is over, i.e. the following condition in MediaToken.sol, line 70, is true:

( recurrentAllowed[from][spender].interval > 0 && block.number >= recurrentAllowed[from][spender].intervalStart + recurrentAllowed[from][spender].interval )

new intervalStart is computed as

newIntervalStart = (block.number / interval) * interval + intervalStart % interval

Let block.number be 1202, interval be 50, intervalStart be 1005. Then, according to the formula, we obtain newIntervalStart as 1205, that is in the future to the current block number (1202). It looks that the formula should use block number difference between the current block and the last intervalStart, i.e. as follows:

newIntervalStart = (block.number — intervalStart) / interval * interval + intervalStart
The issue has been fixed and is not present in the latest version of the code.
  • MediaManager contract logic error

Once a user opted-out from MediaManager services, there is no way to opt-in. Perhaps the contract owner should be able to reset the user’s state.

Comment from the developer:
“This behavior is intentional. The use of MediaManager is temporary, and is needed to support hybrid server till the environment is ready to switch to fully-decentralised solution. Of course, the MediaManager by design is weakening the MediaToken and Mediatoken holders security, so anyone can opt-out some of their account (e.g. the accounts where large amount of tokens is held), while using the hybrid solution from other accounts. The opt-out is permanent. Allowing status reset by the admin, or accidentally by the user is exactly what we wanted to disable. Documentation updated.”

Out of gas

The default function handler reserves 10000 gas for its own execution before calling the delegate. However, check that the available amount of gas exceeds 10000 is not performed, so unsigned integer underflow error is possible. The delegate function may be called with some huge amount of gas reservation.

We recommend adding the check require(gas > 10000).

The issue has been fixed and is not present in the latest version of the code.

Gas limit and loops

The Promotion contract functions contain loops with unknown number of iterations at the following lines:

  • 174
The issue has been fixed: the loop’s length was limited to 10.
  • 214
The issue has been fixed: the loop was removed.
  • 219
Comment from the developer:
“This loops frees some gas with the delete call, that covers the gas consumed in the given iteration.”
  • 237
The issue has been fixed: the loop was removed.
  • 243
Comment from the developer:
“This loops frees some gas with the delete call, that covers the gas consumed in the given iteration.”
  • 283
The issue has been fixed: the visibility of method getReferral was changed to private and the loop’s length was limited to 4.
  • 295
The issue has been fixed: the loop’s length was limited to 4.
  • 409
The issue has been fixed: the loop’s length was limited to 10.

If the arrays are large enough, the functions will not fit in the block gas limit and the transactions calling it will thus never be confirmed. If the array can be influenced by an attacker (e.g., if an attacker can register arbitrary number of investor accounts), this can lead to an attack.

We highly recommend avoiding loops with big or unknown number of steps.

Unchecked Math

Solidity is prone to an integer over- and underflow. Overflow leads to unexpected effects and can lead to loss of funds if exploited by malicious account. It is recommended to use the SafeMath library for all arithmetic operations. It is not used in

  • Promotion.sol, line 253
return ((what + rewardPeriod — 1) / rewardPeriod) * rewardPeriod;
The issue has been fixed: require() was added.
  • Promotion.sol, line 284
value = value * (100 — p.referralDecrease) / 100;
The issue has been fixed: require(referralDecrease <= 100) was added.
  • Promotion.sol, line 349
uint consumerBudget = (budget * (100 — referralShare)) / 100;
The issue has been fixed: require(referralShare <= 100) was added.

Low severity issues

Low severity issues can influence smart contracts operation in future versions of code. We recommend to take them into account.

No return value

The transfer function signature contains the return value type in MediaToken.sol, line 125:

function transfer(address to, uint256 value) public returns (bool){
if(super.transfer(to, value))
ExternalTransfer(msg.sender, to, msg.sender, value);
}

However, there is no return value in the function.

If you don’t need the return value of the function, do not specify returns in the function signature

The issue has been fixed and is not present in the latest version of the code.

Transfer no throw

According to the ERC20 Token Standard, transfer should throw if the _from account balance does not have enough tokens to spend. However, in MediaToken.sol, line 125, transfer throws nothing.

We recommend following the ERC20 Token Standard.

The issue has been fixed and is not present in the latest version of the code.

Unchecked Math

Solidity is prone to an integer over- and underflow. Overflow leads to unexpected effects and can lead to loss of funds if exploited by malicious account. We recommend using the SafeMath library for all arithmetic operations. It is not used in Promotion.sol, lines:

  • 200
The issue has been fixed and is not present in the latest version of the code.
  • 204
Comment from the developer:
“Not an issue, as block number -1 cannot underflow”
  • 211
Comment from the developer
“The if statement limits switchtime to be greater or equal to block number, and newLastProcessedBlock is blocknumber — {0,1} — this statement can hardly underflow.”
  • 218
Comment from the developer:
“The remainingBudgertAllocation is limited by total coins issued, and this is far from uint256 limit.”
  • 222
The issue has been fixed: the checks were added.
  • 223
The issue has been fixed: the checks were added.
  • 242
The issue has been fixed: the checks were added.
  • 253
The issue has been fixed: the check was added.
  • 261
Comment from the developer:
“Allocation is limited by total coins issued, while referral share is less or equal to 100. The result this is far from uint256 limit.”
  • 264
Comment from the developer:
“Allocation is limited by total coins issued, while referral share is less or equal to 100. The result this is far from uint256 limit.”
  • 284
The issue has been fixed and is not present in the latest version of the code.
  • 349
The issue has been fixed and is not present in the latest version of the code.
  • 364
The issue has been fixed and is not present in the latest version of the code.

Using inline assembly

Inline assembly is used in Upgradeable.sol, line 65:

assembly {
// return _dest.delegatecall(msg.data)
calldatacopy(0x0, 0x0, calldatasize)
let ret := delegatecall(sub(gas, 10000), target, 0x0, calldatasize, 0, len)
jumpi(rev, iszero(ret))
return(0, len)
rev: revert(0,0)
}

Inline assembly is a way to access the Ethereum Virtual Machine at a low level. This discards several important safety features of Solidity. We recommend not using assembly if possible.

The developer has updated code according to our recommendations, making this issue secure in current code implementation. However, in future implementations of the code this may lead to vulnerabilities, so we highly recommend paying attention to this pattern.

Implicit visibility level

Functions without a visibility modifier are public (i.e., can be called from outside the contract). Make sure this conforms to the intended functionality. Explicitly define function visibility levels (public, private, external, internal) to improve code readability

(Migrations.sol, line 11, 15, 19)

The issue has been fixed and is not present in the latest version of the code.

Potential Violation of Checks-Effects-Interaction pattern

There is a Checks-Effects-Interaction violation in:

  • Promotion.sol, functions:
  1. getInteraction
  2. processBacklog
  3. processReferralBacklog
  • subscriptionManager.sol, functions:
  1. subscribe
  2. renewSubscription

In these cases the violation do not lead to actual vulnerabilities. However, we highly recommend following best practices including Checks-Effects-Interactions pattern since it helps to avoid many serious vulnerabilities.

Comment from the developer:
“All mentioned cases are trade-offs between user experience (near-real-time payouts, requiring to do transfers in a cycle and cost of transaction). Since the only contracts called are the one in our package (control), it is not an issue.”

Use of selfdestruct

selfdestruct is used in Promotion.sol, line 274

selfdestruct(p.provider);

Using selfdestruct may unexpectedly block calls. Be especially careful if this contract is planned to be used by other contracts (for example, contracts with the library, interactions). Selfdestruction of the contract can leave callers in an inoperable state.

Comment from the developer:
“The selfdestruct is called only once the validity of an individual promotion ends, and after proper clean-up. Any call after the validity and cleanup don’t make sense anyway.”

Hardcoded address

MediaTokenUser.sol contains hardcoded address:

address internal _token = 0x18D6B0208E0425eCe4a046f59342050af7CCBA97;

This address is not currently used on the Ethereum network. We highly recommend checking all the addresses before the deploy.

The issue has been fixed and is not present in the latest version of the code.

Codestyle issues

The Codec smart contracts contain the following codestyle issues:

  • ContentPurchaseTracker contract does not define state variables or implements function. It should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.
  • Delegatable contract meets all requirements of solidity interfaces. It should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.
  • IdentityVerification contract should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.
  • Promotions contract should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.
  • SubscriptionDefinition contract should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.
  • SubscriptionManager contract should be declared as interface.
The issue has been fixed and is not present in the latest version of the code.

Conclusion

In this report we have considered the security of MEDIA Protocol smart contracts. We performed our audit according to the procedure described above.

Our initial audit showed two critical vulnerabilities and several medium and low severity issues. We have discussed the results of the audit with MediaProtocol team. In cases when the developer was aware about possible issues and they were the part of the design we supplied the report with the developer’s comments. All the other issues have been fixed by the developer.

The latest version of the code does not contain confirmed security issues.

This audit was performed by SmartDec, a security team specialized in static code analysis, decompilation and secure development.
Feel free to use SmartCheck, our smart contract security tool for Solidity language, and follow us on Medium. We are also available for smart contract development and auditing work.