PeripheryValidation Manual Review Findings

PVN-01M: Redundant ABI Coder Specification

Type	Severity	Location
Language Specific		PeripheryValidation.sol:L3

Description:

The PeripheryValidation contract does not use any complex ABI instructions and as such does not require the abicoder enabled.

Example:

Copy
1// SPDX-License-Identifier: GPL-2.0-or-later
2pragma solidity ^0.8.17;
3pragma abicoder v2;
4
5abstract contract PeripheryValidation {
6    /// @dev Method that exists purely to be overridden for tests
7    /// @return The current block timestamp
8    function _blockTimestamp() internal view virtual returns (uint256) {
9        return block.timestamp;
10    }
11
12    modifier checkDeadline(uint256 deadline) {
13        require(_blockTimestamp() <= deadline, "Transaction too old");
14        _;
15    }
16    
17    modifier checkPreviousBlockhash(bytes32 previousBlockhash) {
18        require(blockhash(block.number - 1) == previousBlockhash, "Blockhash");
19        _;
20    }
21}

Recommendation:

We advise it to be omitted, reducing the potential compiler-related bug surface of the contract.

Alleviation:

The abicoder statement is no longer present in the codebase as advised.

PVN-02M: Potentially Insecure Hash Evaluation

Type	Severity	Location
Language Specific		PeripheryValidation.sol:L18

Description:

The blockhash of the previous block is an ill-advised security mechanism as blockchains that have transitioned to a PoS system have a significantly small block-time and are susceptible to blockhash manipulations as well as transaction re-ordering.

Impact:

In an actual deployment scenario on a PoS chain, the checkPreviousBlockhash security mechanism will hinder usage of functions and require an abnormally high fee to ensure that the transaction gets executed in time.

Example:

Copy
17modifier checkPreviousBlockhash(bytes32 previousBlockhash) {
18    require(blockhash(block.number - 1) == previousBlockhash, "Blockhash");
19    _;
20}

Recommendation:

We advise the checkPreviousBlockhash protection mechanism to be revisited and potentially omitted as the value acquired from it appears inexistent and could potentially hinder the usage of the system.

Alleviation:

The blockhash based validation system has been removed from the codebase thereby alleviating this exhibit in full.