EVM Smart Contracts - APRO

The audited system, a fork of Chainlink, is designed for deployment across multiple EVM-compatible chains, including Layer 2 (L2) solutions and sidechains. These chains include Scroll, Linea, ZKLink Nova, BSC, and others that are not L2s or L2s. A critical omission has been identified in the implementation: the absence of L2 sequencer feeds contracts on these chains for Apro Contract.

L2 solutions and some sidechains utilize sequencers to batch and process transactions. The operational status of these sequencers is crucial for the integrity and fairness of the oracle service. Without sequencer status monitoring, the system lacks the capability to detect and respond to sequencer downtime or unavailability.

Chainlink's standard implementation includes sequencer feeds to provide real-time status updates of L2 sequencers. These feeds allow oracle consumers to determine whether the L2 network is fully operational and accessible to all users.

The oracle may continue to report prices without accounting for the inaccessibility of the L2 network to most users, potentially leading to mispriced assets or unfair liquidations.

To address this issue, one of the following measures is recommended:

1. Implement L2 Sequencer Feeds: Deploy sequencer status feed contracts on all supported L2 and sidechain networks, following Chainlink's established patterns.

2. Integrate Sequencer Status Checks: Modify the oracle contracts to incorporate checks against the sequencer status feeds before providing price data.

Remediation Plan

PENDING: The APRO team will solve the issue by deploying a L2 Sequencer Feed on Scroll Rollup

Apro contracts use multiple versions of the Solidity compiler pragma, specifically 0.7.6 and ^0.7.0. This inconsistency can lead to compatibility issues, as different pragma versions may have varying behavior and features. The following files use 0.7.6:

• AccessControlledOffchainAggregator.sol

• OffchainAggregator.sol

• OffchainAggregatorBilling.sol

• Owned.sol

• SimpleReadAccessController.sol

• SimpleWriteAccessController.sol

The following files use ^0.7.0:

• AccessControllerInterface.sol

• AggregatorInterface.sol

• AggregatorV2V3Interface.sol

• AggregatorV3Interface.sol

• AggregatorValidatorInterface.sol

• LinkTokenInterface.sol

• TypeAndVersionInterface.sol

• AproTokenInterface.sol

Additionally, the usage of solc-0.7.6 is flagged as not recommended for deployment. It is crucial to use consistent and up-to-date Solidity versions to ensure contract security and compatibility.

It is recommended to use a consistent and up-to-date pragma version across all contracts. It is recommended to upgrade to the latest stable Solidity version (e.g., 0.8.16 or higher) to benefit from the latest security features and improvements.

Remediation Plan

SOLVED : A consistent solidity pragma is now used.

The function decodeReport in the OffchainAggregator contract is identified as unused and effectively dead code because of the internal modifier. This function is designed to decode a report from a byte array into its constituent components: rawReportContext, rawObservers, and observations. The code is as follows:

function decodeReport(bytes memory _report) internal pure
  returns (
    bytes32 rawReportContext,
    bytes32 rawObservers,
    int192[] memory observations
  )
{
  (rawReportContext, rawObservers, observations) = abi.decode(_report,(bytes32, bytes32, int192[]));
}

The decodeReport function is intended for internal use but is never called within the contract, making it dead code.

It is recommended to remove the decodeReport function from the contract to streamline the codebase and eliminate unnecessary code.

Remediation Plan

SOLVED : The APRO team removed the unused function.

During the security assessment, it was identified that the project uses an outdated version of Solidity (pragma solidity ^0.7.0;). This version was released on July 28, 2020, and has several known bugs that could potentially impact the security and functionality of the smart contracts.

The known bugs in Solidity version 0.7.0 include:

• KeccakCaching

• AbiReencodingHeadOverflowWithStaticArrayCleanup

• InlineAssemblyMemorySideEffects

• DynamicArrayCleanup

• FreeFunctionRedefinition

These bugs can lead to unexpected behavior or exposure to known security vulnerabilities.

It is strongly recommended to upgrade to the latest stable version of Solidity to mitigate the risks associated with these known bugs. By upgrading, the project can benefit from the latest security patches, improvements, and new features introduced in the newer versions of Solidity.

Remediation Plan

ACKNOWLEDGED: The APRO team acknowledged the finding.

When the length of an array is not cached outside of a loop, the Solidity compiler reads the length of the array during each iteration. For storage arrays, this results in an extra sload operation (100 additional gas for each iteration except the first). For memory arrays, this results in an extra mload operation (3 additional gas for each iteration except the first).

For example :

for (uint i = 0; i < _signers.length; i++) { // add new signer/transmitter addresses
      require(
        s_oracles[_signers[i]].role == Role.Unset,
        "repeated signer address"
      );
      s_oracles[_signers[i]] = Oracle(uint8(i), Role.Signer);
      require(s_payees[_transmitters[i]] != address(0), "payee must be set");
      require(
        s_oracles[_transmitters[i]].role == Role.Unset,
        "repeated transmitter address"
      );
      s_oracles[_transmitters[i]] = Oracle(uint8(i), Role.Transmitter);
      s_signers.push(_signers[i]);
      s_transmitters.push(_transmitters[i]);
    }

It is recommended to cache the length of the array in a local variable outside the loop to optimize gas usage. This reduces the number of read operations required during each iteration of the loop.

Remediation Plan

ACKNOWLEDGED: The APRO team acknowledged the finding.

The EACAggregatorProxy.sol does not include an SPDX license identifier on the first line. The SPDX identifier is a recommended standard that specifies the software license governing the code, ensuring clarity and compliance with legal requirements.

pragma solidity 0.7.6;

/**
 * @title The Owned contract
 * @notice A contract with helpers for basic contract ownership.
 */
contract Owned {
    ...
}

The absence of an SPDX license identifier can lead to potential legal ambiguities. Without a clear license declaration, users and developers may be uncertain about the usage, modification, and distribution rights of the contract. This can hinder collaboration and adoption, and may lead to legal complications if the code is used or modified without proper licensing information.

To adhere to best practices and legal standards, it is recommended to include an SPDX license identifier at the beginning of the contract file. This provides clear information about the licensing terms under which the code is distributed.

By adding the SPDX license identifier, you ensure that the licensing information is explicitly communicated, reducing legal risks and promoting better code practices.

Remediation Plan

SOLVED: The APRO team added the SPDX License Identifier to solve the issue.

The smart contract under review contains an interface named LinkTokenInterface in file AproTokenInterface.sol which is a direct copy from the Chainlink project. However, this project is named APRO, and the interface name has not been updated to reflect this fact. The current implementation is as follows:

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.0;

interface LinkTokenInterface {
// ... (interface functions)
}

This naming inconsistency is present throughout the interface, including comments that directly reference Chainlink:

// The client constructs and makes a request to a known Chainlink oracle through the transferAndCall function, implemented by the LINK token

To address this issue, the following changes are recommended:

1. Rename the interface to reflect the APRO project

2. Update all references to Chainlink in comments and documentation within the interface

Remediation Plan

SOLVED : Naming conventions has been modified.