CowswapOrderSigner Static Analysis Findings

COS-01S: Illegible Numeric Value Representation

Type	Severity	Location
Code Style		CowswapOrderSigner.sol:L36

Description:

The linked representation of a numeric literal is sub-optimally represented decreasing the legibility of the codebase.

Example:

Copy
36order.feeAmount <= (order.sellAmount * feeAmountBP) / 10000 + 1,

Recommendation:

To properly illustrate the value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation (da3062f6b3):

The underscore separator has been introduced after the second digit (10_000) rather than the decimal point (100_00), rendering this exhibit partially applied.

Alleviation (50bc415e93):

The underscore separator has been introduced at the correct point of the numeric literal, addressing this exhibit correctly.

COS-02S: Inexistent Visibility Specifiers

Type	Severity	Location
Code Style		CowswapOrderSigner.sol:L16, L17

Description:

The linked variables have no visibility specifier explicitly set.

Example:

Copy
16bytes32 immutable domainSeparator;

Recommendation:

We advise them to be set so to avoid potential compilation discrepancies in the future as the current behaviour is for the compiler to assign one automatically which may deviate between pragma versions.

Alleviation (da3062f6b3ff452092a0b6daa6f226f0f3b696c6):

The public visibility specifier has been introduced to all referenced variables, preventing potential compilation discrepancies and addressing this exhibit.

COS-03S: Inexistent Sanitization of Input Address

Type	Severity	Location
Input Sanitization		CowswapOrderSigner.sol:L19-L23

Description:

The linked function accepts an address argument yet does not properly sanitize it.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
19constructor(GPv2Signing _signing) {
20    signing = _signing;
21    domainSeparator = _signing.domainSeparator();
22    deployedAt = address(this);
23}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that the address specified is non-zero.

Alleviation (da3062f6b3ff452092a0b6daa6f226f0f3b696c6):

The input _signing address argument of the CowswapOrderSigner::constructor function is adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.