[Low-01] Missing Event Emission in setUpdateWeightRunnerAddress Function#L115-L119

Affected Code:

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/interfaces/contracts/pool-quantamm/IQuantAMMWeightedPool.sol#L115-L119

Vulnerable Code:
function setUpdateWeightRunnerAddress(address _updateWeightRunner) external {
}

Description and Impact:

• Lack of Transparency: Off-chain systems cannot detect when the function is called, making it harder to monitor critical address changes.

• Security Risk: No logs to track address changes, which could result in unnoticed misuse or malicious actions.

• Debugging Challenge: Absence of event logs makes it harder to debug and verify operational correctness.

Recommended Code (Fixed Code):

function setUpdateWeightRunnerAddress(address _updateWeightRunner) external {
// Logic for updating the runner...
emit UpdateWeightRunnerAddressSet(_updateWeightRunner); // Emit event when the address is updated
}

// Add the corresponding event declaration
event UpdateWeightRunnerAddressSet(address indexed updateWeightRunner);

[Low-02] Missing Events and Zero Address Check in LPNFT Contract#L34-L50

Affected Code:

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/LPNFT.sol#L34-L50

Description:

The LPNFT contract is a basic implementation of an ERC721 NFT for LP tokens with specific modifications for minting and burning operations. The contract currently lacks events for tracking important actions like minting and burning NFTs. Additionally, there is a potential issue where the mint function could mint tokens to the zero address, which is an invalid operation in ERC721.

Impact:

• Missing Events:

  • Transparency: Without events, off-chain systems like dApps and monitoring tools cannot detect when NFTs are minted or burned.

  • Accountability: Without logs, it’s difficult to identify who minted or burned specific NFTs.

• Zero Address Check Missing:

  • Security Risk: Minting to the zero address could result in tokens being trapped, inaccessible, or unusable, which could break ownership and contract functionality.

Vulnerable Code:

solidity

Copy code

/// @param _to the address to mint the NFT to
function mint(address _to) public onlyUpliftOnlyRouter returns (uint256 tokenId) {
tokenId = ++numMinted; // We start minting at 1
_mint(_to, tokenId);
}

/// @param _tokenId the id of the NFT to burn
function burn(uint256 _tokenId) public onlyUpliftOnlyRouter {
_burn(_tokenId);
}

Recommended Code (Fixed Code):

solidity

Copy code

// Event declarations
event Mint(address indexed to, uint256 tokenId);
event Burn(address indexed from, uint256 tokenId);

/// @param _to the address to mint the NFT to
function mint(address _to) public onlyUpliftOnlyRouter returns (uint256 tokenId) {
require(_to != address(0), "Mint to the zero address is not allowed.");
tokenId = ++numMinted; // We start minting at 1
_mint(_to, tokenId);
emit Mint(_to, tokenId); // Emit event for mint
}

/// @param _tokenId the id of the NFT to burn
function burn(uint256 _tokenId) public onlyUpliftOnlyRouter {
address owner = ownerOf(_tokenId);
_burn(_tokenId);
emit Burn(owner, _tokenId); // Emit event for burn
}

[Low-03] Missing Zero Address Check in Constructor of PowerChannelUpdateRule#L12-L14

Affected Code:

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-quantamm/contracts/rules/AntimomentumUpdateRule.sol#L12-L14

Description:

The constructor of the PowerChannelUpdateRule contract does not check if the _updateWeightRunner address is the zero address (address(0)). The _updateWeightRunner address is passed to the parent contract UpdateRule without validation, which can result in unintentional behavior if the address is set to the zero address.

Impact:

• Security Risk: If the _updateWeightRunner address is set to address(0), the contract may fail to interact with the proper address, causing access control bypass or failure in critical functions.

• Contract Functionality: The contract may not perform necessary operations, potentially breaking logic or causing transaction failure.

• Unintended Behavior: If the zero address is used, functions depending on this address may fail, potentially breaking the contract's intended flow.

Vulnerable Code:

solidity

Copy code

constructor(address _updateWeightRunner) UpdateRule(_updateWeightRunner) {
name = "PowerChannel";
}

Recommended Code (Fixed Code):

solidity

Copy code

constructor(address _updateWeightRunner) UpdateRule(_updateWeightRunner) {
require(_updateWeightRunner != address(0), "Invalid address: zero address not allowed");
name = "PowerChannel";
}

[Low-04] Missing Zero Address Check in setUpdateWeightRunnerAddress Function#L115-L119

Affected Code:

https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/interfaces/contracts/pool-quantamm/IQuantAMMWeightedPool.sol#L115-L119

Description: The function setUpdateWeightRunnerAddress is responsible for updating the address of the updateWeightRunner in the contract. However, it currently lacks a validation check to ensure that the input address _updateWeightRunner is not the zero address (address(0)).

Impact:

• Allowing the zero address to be set could cause the contract to malfunction, causing errors or unexpected behavior.

• Potential misconfiguration, resulting in failure of important operations related to the weight runner.

Vulnerable Code:

solidity

Copy code

/// @inheritdoc IQuantAMMWeightedPool
function setUpdateWeightRunnerAddress(address _updateWeightRunner) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
updateWeightRunner = UpdateWeightRunner(_updateWeightRunner);
emit UpdateWeightRunnerAddressUpdated(address(updateWeightRunner), _updateWeightRunner);
}

Affected Code Line:

• Lines 817-821

Recommended Code (Fixed Code):

solidity

Copy code

/// @inheritdoc IQuantAMMWeightedPool
function setUpdateWeightRunnerAddress(address _updateWeightRunner) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_updateWeightRunner != address(0), "INVALID_ADDRESS"); // Ensure non-zero address
updateWeightRunner = UpdateWeightRunner(_updateWeightRunner);
emit UpdateWeightRunnerAddressUpdated(address(updateWeightRunner), _updateWeightRunner);
}