Read the findings of the 2022 Web3 Security Report

In the contract, `PYESwapPair`, the variable `totalFee` is defined, which can be set to any value using the `updateTotalFee()` function. However, in the file `PYESwapRouter.sol` this variable will be used for example to:
- compute fees, in this case, if `totalFee > 10000` then the fees will be greater than 100%;
- compute `amountInMultiplier` and `amountOutMultiplier`, in this case, if `totalFee > 10000` an underflow could happen and assign huge values to these two variables.

`totalFee` Missing Upper Limit

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects.

Check Effect Interaction Pattern Violated

Addresses should be checked before assignment or external calls to make sure they are not zero addresses.

Missing Zero Address Validation

The contract is relying on functions whose implementations are unknown. The scope of the audit treats these entities as black boxes and assumes their functional correctness.

Unknown Implementations

In the contracts `PYESwapRouter.sol` and `PYESwapFactory.sol` multiple `solidity` versions are used in the codebase :
- `=0.6.6`,
- `>=0.5.0`,
- `>=0.6.0`,
- `>=0.6.2`,
- `=0.5.16`,
- `>=0.5.16`

-----
Many of these versions are also unlocked. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to different compiler versions. This can lead to ambiguity when debugging as compiler-specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Different Solidity Versions | Unlocked Compiler Version

There should always be events emitted in the sensitive functions, especially the ones that are controlled by centralization roles.

Missing Emit Events

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.

Variables That Could Be Declared as Immutable

The return value of an external call is not stored in a local or state variable.

Unused Return Value

In the contract `PYESwapPair`, the function `initialize()` can be called by the `factory` more than once, allowing the privileged account to change the tokens.

Function `initialize()` Can Be Called More Than Once

In the contract `PYESwapPair`, in the `swap()` function:
\
the variable `amount0In` is set as follow: 
- if `balance0 > _reserve0 - amount0Out - amount0Fee` :
    * then `amount0In = balance0 - (_reserve0 - amount0Out)`,
    * else `amount0In = 0`.

This means that it is possible to have `(_reserve0 - amount0Out) > balance0` because the condition checked is equivalent to :
- `balance0 + amount0Fee > _reserve0 - amount0Out`.

ultimately this could lead to an underflow of `balance0 - (_reserve0 - amount0Out)` ending with `amount0In` having a ridiculous value.

----

The `amount1In` variable is computed the exact same way.

Logical Issue in The Function `swap()`

In the contract `PYESwapFactory` the role `feeToSetter` has authority over the functions shown in the diagram below.

Any compromise to the `feeToSetter` account may allow the hacker to take advantage of this authority and :
- set the `feeToSetter` to an address he/she controls;
- set the `feeTo` address to an address he/she controls so the hacker can divert the minted fees. 

![](https://accelerator-tasks-prod.s3.amazonaws.com/467ee7876ed74ada868c9da80322f56f/diagrams/centralization-PYESwapFactory-feeToSetter.svg)

In the contract `PYESwapPair` the role `factory` has authority over the functions shown in the diagram below.

Any compromise to the `factory` account may allow the hacker to take advantage of this authority and, for example:
- reinitialize the factory contract to change the `token0` and `token1` variables of the contract;
- set or change the `baseToken`.

![](https://accelerator-tasks-prod.s3.amazonaws.com/467ee7876ed74ada868c9da80322f56f/diagrams/centralization-PYESwapPair-factory.svg)

In the contract `PYESwapPair` the role `feeTaker` has authority over the function shown in the diagram below.

Any compromise to the `feeTaker` account may allow the hacker to take advantage of this authority and change the value of the `totalFee` which could seriously interfere with the logic of the system.

![](https://accelerator-tasks-prod.s3.amazonaws.com/467ee7876ed74ada868c9da80322f56f/diagrams/centralization-PYESwapPair-feeTaker.svg)

Centralization Risk in `PYESwapFactory.sol`

The call to the `burn()` function will fail if the value of `totalSupply` is 0, due to a division by zero.

Divide by Zero

The signature malleability is possible within the Elliptic Curve cryptographic system. An Elliptic Curve is symmetric on the X-axis, meaning two points can exist with the same `X` value. In the `r`, `s`, and `v` representation this permits us to carefully adjust `s` to produce a second valid signature for the same `r`, thus breaking the assumption that a signature cannot be replayed in what is known as a replay attack.

Susceptible to Signature Malleability

In the contract `PYESwapFactory`, the `routerInitialize()` function can be called by anyone as long as it is the first time this function is called. Since all the transactions on the blockchain are public, a malicious user could try to take advantage of `PYESwapFactory` being deployed with an uninitialized router, to set an address he/she controls as the `routerAddress`.

No Restriction On The Caller Of `routerInitialize()`

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

Usually, a DEX based on the constant product formula uses a safety check after a swap to be sure that the trade did not leave the trading pair with less reserves than it should be.

Here, in the contract `PYESwapPair`, the `swap()` function does not perform any check. Is that intended?

`swap()` Function Does Not Perform Safety Check About The Product Of The Reserves

In the contract `Ownable` the role `_owner` has authority over the functions shown in the diagram below:

![](https://accelerator-tasks-prod.s3.amazonaws.com/2720b4b1be03403e9b492fd35e98bdaa/diagrams/centralization-Ownable-_owner.svg)

In the contract `FeeStore` the role `_owner` has authority over the function shown in the diagram below:

![](https://accelerator-tasks-prod.s3.amazonaws.com/2720b4b1be03403e9b492fd35e98bdaa/diagrams/centralization-FeeStore-_owner.svg)

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and for example:
- transfer the ownership to an address he/she controls;
- set an address he/she controls as the `adminFeeAddress` and change the `adminFee` value, to divert tokens during swaps.

Centralization Risk in `PYESwapRouter.sol`

In the function `_addLiquidity()`, if the pair does not exist yet, the function will call `createPair()` from `IPYESwapFactory`to create it. However if `(tokenA == BUSD)` then the pair is not created through the call of this function.

Pair Is Not Created If `(tokenA == BUSD)`

The return value of the linked `transferFrom()` call is not checked.

Unchecked ERC-20 `transferFrom()` Call

In the contracts `FeeStore` and `SupportingSwap`, the variable `adminFee` is used to compute a specific fee. If `adminFee > 10000` then every time `adminFee` is used the execution will revert. However, nothing prevents setting `adminFee` as a number greater than 10000.

`adminFee` Should Be Inferior To 10000

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks.

Pyeswap

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

21

0

2

2

17