Traxx security record

During the creation of a key, the quantity of tokens for the corresponding tier is incremented in the `quantitySold` variable while also ensuring that the number of tokens sold does not exceed the `totalSupply`. However, when the burn function is called, the `quantitySold` variable is not updated. As a result, it is possible that an artist may have a certain tier of tokens available for purchase but cannot sell them due to the `quantitySold` variable not being updated.

The following error shows when the `TokenTraxx` contract in the `traxx-protocol` repo is compiled. ```bash error[9098]: TypeError: Overriding function visibility differs. --> src/contracts/TokenTraxx.sol:174:5: | 174 | function revokeRole(bytes32 role, address account) | ^ (Relevant source part starts here and spans across multiple lines). Note: Overridden function is here: --> node_modules/@openzeppelin/contracts/access/AccessControl.sol:159:5: | 159 | function revokeRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) { | ^ (Relevant source part starts here and spans across multiple lines). error[4327]: TypeError: Function needs to specify overridden contracts "AccessControl" and "IAccessControl". --> src/contracts/TokenTraxx.sol:176:9: | 176 | override | ^^^^^^^^ Note: This contract: --> node_modules/@openzeppelin/contracts/access/AccessControl.sol:49:1: | 49 | abstract contract AccessControl is Context, IAccessControl, ERC165 { | ^ (Relevant source part starts here and spans across multiple lines). Note: This contract: --> node_modules/@openzeppelin/contracts/access/IAccessControl.sol:9:1: | 9 | interface IAccessControl { | ^ (Relevant source part starts here and spans across multiple lines). error[9098]: TypeError: Overriding function visibility differs. --> src/contracts/TokenTraxx.sol:205:5: | 205 | function renounceRole(bytes32 role, address account) | ^ (Relevant source part starts here and spans across multiple lines). Note: Overridden function is here: --> node_modules/@openzeppelin/contracts/access/AccessControl.sol:179:5: | 179 | function renounceRole(bytes32 role, address account) public virtual override { | ^ (Relevant source part starts here and spans across multiple lines). error[4327]: TypeError: Function needs to specify overridden contracts "AccessControl" and "IAccessControl". --> src/contracts/TokenTraxx.sol:208:9: | 208 | override | ^^^^^^^^ Note: This contract: --> node_modules/@openzeppelin/contracts/access/AccessControl.sol:49:1: | 49 | abstract contract AccessControl is Context, IAccessControl, ERC165 { | ^ (Relevant source part starts here and spans across multiple lines). Note: This contract: --> node_modules/@openzeppelin/contracts/access/IAccessControl.sol:9:1: | 9 | interface IAccessControl { | ^ (Relevant source part starts here and spans across multiple lines). ```

The `safeTransferERC20` function is intended to transfer an ERC20 token from one address to another. However, the function may not be compatible with deflationary tokens, as the require statement may not accurately reflect the success of the transfer. The `require` statement in the function is checking if the balance after the transfer is equal to the balance before the transfer plus the amount being sent. This check may not work correctly with deflationary tokens, as they often have a built-in fee mechanism that reduces the amount being transferred. In such cases, the balance after the transfer will not be equal to the balance before the transfer plus the amount being sent, leading to a failed transfer and a potential loss of funds for the user. ```solidity=781 require(balAfter == balBefore + _amount, throwError(TRANSFER_FAILED)); ```

The `removeListing` function allows the admin to remove a listing by calling the `pop()` function on the `_tokenListings[assetContract][tokenId]` array. This function removes the latest element of the array, which may not be equal to the `listingId` specified in the function call. This can result in incorrect deletion of elements from the `_tokenListings` array and can cause unintended behavior in the contract. ```solidity=1162 address assetContract = listings[listingId].assetContract; uint256 tokenId = listings[listingId].tokenId; // Iterate over listing Ids when supporting secondary sale and multiple listings can exist require( listings[listingId].assetContract != address(0), throwError(LISTING_ID_NOT_FOUND) ); _tokenListings[assetContract][tokenId].pop(); delete listings[listingId]; emit ListingRemoved(listingId); ```

There could be issue in the calculation of the `_transferAmount` and remainder when the `isDiscounted` flag is `false` and the `_listing.saleType` is `SaleType.Secondary`. This issue could lead to funds being left in the contract instead of being completely distributed to the involved parties, which may result in lost funds for users participating in secondary sales. 1. If `isDiscounted` is `false` and `_listing.saleType` is `SaleType.Secondary`, the `marketCut` is calculated as follows: - `marketCut = (_totalPayoutAmount * marketFeeBps) / MAX_BPS` 2. The `remainder` is calculated: - `remainder = _totalPayoutAmount - marketCut` 3. The code then tries to fetch royalty information and calculates the `royaltyFeeAmount`. If `royaltyFeeAmount` is greater than 0, the following checks and updates are made: - Ensure that the sum of royaltyFeeAmount and marketCut does not exceed _totalPayoutAmount: `require(royaltyFeeAmount + marketCut <= _totalPayoutAmount, throwError(FEES_EXCEEDS_PRICE))` - Update the remainder: `remainder -= royaltyFeeAmount` - Update the _transferAmount: `_transferAmount += royaltyFeeAmount` At this point, `_transferAmount` does not include the `marketCut`, which means that the sum of `_transferAmount` and `remainder` might not equal `_totalPayoutAmount`. This could result in funds being left in the contract when users use the native token for paying. ```solidity=665 uint256 marketCut; uint256 _transferAmount; if (!_isDiscounted) { if (_listing.saleType == SaleType.Primary) marketCut = (_totalPayoutAmount * primaryFeeBps) / MAX_BPS; else marketCut = (_totalPayoutAmount * marketFeeBps) / MAX_BPS; } uint256 remainder = _totalPayoutAmount - marketCut; if (_listing.saleType == SaleType.Secondary) { // Distribute royalties. See Sushiswap's https://github.com/sushiswap/shoyu/blob/master/contracts/base/BaseExchange.sol#L296 try IERC2981(_listing.assetContract).royaltyInfo( _listing.tokenId, _totalPayoutAmount ) returns (address royaltyFeeRecipient, uint256 royaltyFeeAmount) { if (royaltyFeeAmount > 0) { require( royaltyFeeAmount + marketCut <= _totalPayoutAmount, throwError(FEES_EXCEEDS_PRICE) ); remainder -= royaltyFeeAmount; _transferAmount += royaltyFeeAmount; tShares[royaltyFeeRecipient][ _currencyToUse ] += royaltyFeeAmount; } } catch {} } else { _transferAmount += marketCut; tShares[royaltyTreasury][_currencyToUse] += marketCut; } ```

The `validateSaleParameters` function checks if the sum of the current primary sale user count and the requested purchase quantity is within the allowed purchase limit using the following condition: `collectionContractInstance.getPrimarySaleUserCount( msg.sender, collectionId) + quantityToBuy <= uint256(collectionData.userPurchaseLimit)`. However, due to the missing quantity parameter when calling `updateCollectionAfterPrimarySale()` in the `executeSale()` function, the primary sale user count may not be updated correctly, allowing users to bypass the purchase limit. ```solidity=428 if ( INFTCustom(listingData.assetContract).getFirstOwner( listingData.tokenId ) == collectionData.owner ) { if (collectionData.userPurchaseLimit != -1) { require( collectionContractInstance.getPrimarySaleUserCount( msg.sender, collectionId ) + quantityToBuy <= uint256(collectionData.userPurchaseLimit), throwError(EXCEEDED_PRIMARY_PURCHASE_LIMIT) ); } } ``` ```solidity=574 if ( INFTCustom(_targetListing.assetContract).getFirstOwner( _targetListing.tokenId ) == collectionData.owner ) { collectionContractInstance.updateCollectionAfterPrimarySale( msg.sender, collectionId ); } ```

The signature used for the `Authenticator` contract could also be vulnerable to cross-chain replay attacks. As there is no distinction between the different blockchains supported, transactions originally intended to be executed on one network could also potentially be executed on another network through a replay attack. A potential solution for cross-chain replay attacks is to enforce EIP712 to identify and place checks on the `chainId` to ensure that signatures are only valid for one specific network.

A state variable is shadowing another component defined in a parent contract. Variable `_tokenURIs` in `TokenTraxxNFT1155` shadows the variable `_tokenURIs` in `ERC1155URIStorageUpgradeable`. ```solidity=79 mapping(uint256 => string) private _tokenURIs; ``` ```solidity=33 mapping(uint256 => string) private _tokenURIs; ``` --- Variable `_baseURI` in `TokenTraxxNFT1155` shadows the variable `_baseURI` in `ERC1155URIStorageUpgradeable`. ```solidity=82 string private _baseURI; ``` ```solidity=30 string private _baseURI; ```

The current implementation of the `ICollection` interface allows for potential conflicts between token IDs of different ERC721 and ERC1155 contracts. This is due to the `getERC721CollectionId()` and `getERC1155CollectionId()` functions in the interface only accepting a single parameter, `listingData.tokenId`, which may not be unique across different contracts. Token ID conflicts between different contracts may lead to incorrect behavior, such as the wrong assets being associated with a listing, incorrect ownership tracking, or other unexpected results. This may also expose the contract to potential manipulation by malicious users. ```solidity=382 uint256 collectionId = listingData.tokenType == TokenType.ERC721 ? collectionContractInstance.getERC721CollectionId( listingData.tokenId ) : collectionContractInstance.getERC1155CollectionId( listingData.tokenId ); ```

The comment statement contains several typos. - `contarct` --> `contract` - `contarct` --> `contract` - `addres` --> `address` - `upgarde` --> `upgrade` - `shouldm` --> `should` - `to tranfer the token on successfull purchase` --> `to transfer the token on successful purchase` - `listing of of ERC721 and ERC1155 token` --> `listing of ERC721 and ERC1155 tokens` - `is be transferred` --> `is being transferred` - `iwhether` --> `whether` - `addres` --> `address` - `markeplace` --> `marketplace` - `to be withdrawed` --> `to be withdrawn` - `the maintaner role` --> `the maintainer role` - `array's` --> `arrays` - `quatities` --> `quantities` - `implementaion` --> `implementation` - `synbol` --> `symbol` - `intial tokems` --> `initial tokens` - `logc` --> `logic` - `acccount` --> `account` - `removes` --> `removed` - `doesnt` --> `doesn't` - `Legths` --> `Lengths` - `shouldnt` --> `shouldn't`