Users cant withdraw S1 or S2 Citizens if no timelockEndTime is associated with it

Lines of code Vulnerability details Impact Users cant withdraw S1 or S2 Citizens if no timelockEndTime is associated with it. As the same logic is applied in the withdrawLP function, this is not intended behavior. Currently, there are no allowances for a S1 or S2 Citizen to withdraw their asset i...

NeoTokyoStaker.getPoolReward function can be frontrun, which can cause staker and DAO to lose reward shares that they are entitled to

Lines of code Vulnerability details Impact When calling the following NeoTokyoStaker.stakeBytes and NeoTokyoStaker.stakeLP functions, the higher the specified amount to be staked is, the higher the pool.totalPoints is increased by. function stakeBytes uint256 private uint256 amount; uint256...

Rewards calculation is unfair and leads to stakers losing rewards

Lines of code Vulnerability details User rewards are updated upon staking actions ie stake or withdraw: File: contracts/staking/NeoTokyoStaker.sol 1225: // Grant the caller their total rewards with each staking action. 1226: IByteContractBYTES.getRewardmsg.sender; Which are computed as follows:...

Infinite mint via points underflow (in scope)

Lines of code Vulnerability details Impact Due to unchecked math in the withdrawLP function, a user can trigger an underflow in their points and infinitely increase their rewards. The problem exists in several places. Problem 1. The configureTimelockOptions function allows setting...

wrong updating lp position

Lines of code Vulnerability details Impact lock lp token at staking more than required when user adds some lp token staking amount, contract uses new timelockEndTime for previously staked amount . Proof of Concept Scenario : user have some amount of lp token staked for a period of time like 30 da...

Executing getCreditYield(citizenId, citizenVaultId) in NeoTokyoStaker._stakeS1Citizen function can cause identityPoints to be lower than it should be when corresponding S1 Citizen to be staked does not have a component vault but an optional vault is provided

Lines of code Vulnerability details Impact Calling the following NeoTokyoStaker.stakeS1Citizen function executes string memory citizenCreditYield = getCreditYieldcitizenId, citizenVaultId and uint256 identityPoints = identityCreditPointscitizenCreditYield. When the S1 Citizen to be staked does no...

Lack of a contract existence check may lead to undesired behavior

Lines of code Vulnerability details Impact The functions NeoTokyoStaker.assetTransferFrom and NeoTokyoStaker.assetTransfer are both used as helper functions to perform low level calls, both functions revert if the low-level call fails, but no checks are made if the contract that's being called...

# configureLP function should check LP stakers present before changing LP address.

Lines of code Vulnerability details configureLP function should check LP stakers existence before changing LP address. Permitted users are allowed to change LP address when lpLocked is false. So this does not follow the comments above. Proof of concept 1701 This function allows a permitted user t...

User can call getReward multiple times causing 51% attack

Lines of code Vulnerability details Impact The Neo Tokyo staking program operates as follows: The staker is a competitive system where stakers compete for a fixed emission rate in each of the S1 Citizen, S2 Citizen, and LP token staking pools. Stakers "may" choose to lock their assets for some...

share underflow in getPoolReward()

Lines of code Vulnerability details Impact It is important to draw attention to an issue that, while out of scope, can lead to a complete loss of funds on the contract and, therefore, seems significant. The configurePools function allows setting an arbitrary, unrestricted daoTax, which can result...

TIMELOCK OPTION'S DURATION AND MULTIPLIER INACCURATELY DECODED

Lines of code Vulnerability details Impact The implementation of bitwise operations, i.e. and & in decode the timelock option's duration and multiplier does not seem to return results as expected. This could affect all other variables dependent on them. Proof of Concept These affect the function...

Wrong accounting of share leading to incorrect amount of BYTES be minted per second

Lines of code Vulnerability details Impact In NeoTokyoStaker, staker is a competitive system where stakers compete for a fixed emission rate in each of the S1 Citizen, S2 Citizen, and LP token staking pools. For each staking pool, there are some reward windows. Each reward window has different...

Staking BYTES to Citizen does not extend timelock, allowing attacker to manipulate totalPoints with flash loan

Lines of code Vulnerability details Impact In NeoTokyoStaker, BYTES token can be staked into a Citizen. First, the Citizen must be staked, it will be locked for a timelock duration in Staking contract. Staker want to stake BYTES can specify this Citizen ID and stake into it. However, when users...

the S1 Citizen and S2 Citizen loops in the second getStakerPositions are not working properly

Lines of code Vulnerability details Impact This can result in the second loop not running correctly and resulting in an error in the logic, in case if the length of the stakerS1Positionstaker or stakerS2Positionstaker arrays is very large, the operation of adding or subtracting the index on...

Current transfer implementations can lead to loss of user founds if LP is updated or wrongly set

Lines of code Vulnerability details Current transfer implementations can lead to loss of user founds if LP is updated or wrongly set The current implementation of assetTransfer and assetTransferFrom don't check if the asset is a contract and don't check if the transfer returns a false instead of...

Possible loss of rewards when staking non-component vaults

Lines of code Vulnerability details Impact Users could gain less rewards if they stake a non-component vault, as compared to reassembling their S1 Citizen with the vault and and staking it as a component vault. Vulnerability Details From the repository's README.md: S1 Citizens without a component...

ZERO TRUNCATION COULD LEAD TO UNEXPECTED RESULTS

Lines of code Vulnerability details Impact Precision issue leading to zero truncation due to numerator smaller than denominator in a ratio or a division happens readily in Solidity if extra cares have not been given to it. Arithmetic operations running into this incident are typically associated...

Unclaimed rewards will be stuck unrecoverable

Lines of code Vulnerability details Impact The vulnerability relies in: unchecked uint256 share = points PRECISION / pool.totalPoints totalReward; uint256 daoShare = share pool.daoTax / 100 DIVISOR; share /= PRECISION; daoShare /= PRECISION; return share - daoShare, daoShare; The problem is that ...

Ability to receive LP rewards without having any LP staked

Lines of code Vulnerability details Impact The impact of this is high as a user is able to first stake LP tokens, then craftily withdraw them in specific increments without any change to their staking rewards. The user is able to get to a state in which they have 0 LP tokens staked, but have 0 LP...

Misconfiguration of LP token contract

Lines of code Vulnerability details Impact If the LP token contract is set to a non-contract address or a no-revert-on-transfer token, users will be able to: 1. Mint huge amounts of BYTES 2.0 tokens. 2. Drain the contract of all its LP tokens. Vulnerability Details Throughout the contract, it is...

Huge over calculating user rewards

Lines of code Vulnerability details Impact Huge over calculating user rewards When user claim rewards for the first time rewards over calculated Proof of Concept getPoolReward function uses ''uint256 timeSinceReward = block.timestamp - lastRewardTime'' formula to calculate period of time that...

Updating a pool's total points doesn't affect existing stake positions for rewards calculation

Lines of code Vulnerability details Impact Staking rewards are calculated based on the user's share of total points in the corresponding asset pool, this is the sum of the points associated to the staker's positions divided by the total points from all positions in the pool. We can see this...

Upgraded Q -> 2 from #345 [1678798679676]

Judge has assessed an item in Issue 345 as 2 risk. The relevant finding follows: Issue 2 - Undermining the fairness of the protocol in swapSource and possibilities for stealing a jackpot --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #345 [1678798666534]

Judge has assessed an item in Issue 345 as 2 risk. The relevant finding follows: Issue 1 - An attacker can leave the protocol in a "drawing" state for extended period of time --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #493 [1678589994012]

Judge has assessed an item in Issue 493 as 2 risk. The relevant finding follows: L-02 --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #29 [1678626286403]

Judge has assessed an item in Issue 29 as 2 risk. The relevant finding follows: Maximum fixed reward is $6553.5 - too small in case of selectionSize = 16 --- The text was updated successfully, but these errors were encountered: All reactions...

Return value unchecked - leads to privilege escalation

Lines of code Vulnerability details Impact The application doesn't check the return value of isGrantes properly Proof of Concept no check on the return value only exception handled Tools Used slither Recommended Mitigation Steps check the return value or force it for the implementing class --- Th...

MerkleMinter created through TokenFactory cannot be upgraded

Lines of code Vulnerability details Impact During the token creation process in the TokenFactory contract, the function creates a MerkleMinter contract to setup and handle token initial token distribution. ... // Clone and initialize a MerkleMinter address merkleMinter = merkleMinterBase.clone;...

Not having enough ETH may cause the transaction to fail

Lines of code Vulnerability details Impact The transaction will fail due to not having enough ETH. Proof of Concept When the proposal is executed, the protocol will call the function DAO.execute.The DAO protocol also supports sending ETH to external contract as the following code. bool success,...

Incorrect EIP1271 magic value returned from DAO.isValidSignature

Lines of code Vulnerability details Impact An unexpected return value from the EIP1271 signature verification can lead to authorization for unwanted operations in external protocols. Proof of Concept As shown in the EIP1271 standard specification, when a signature is not valid, the returned magic...

createDao is suspicious of the reorg attack

Lines of code Vulnerability details Description The createDao function creates a dao contract via create opcode. The trace is createDao = createDAO=createERC1967Proxy. The address of the newly created contract depends on the DAOFactory nonce. At the same time, block reorg may happen on any...

Missing 0 address check

Lines of code Vulnerability details Impact A 0 address can be used here an transaction will not revert loosing funds Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. The Function alone doesn't...

DAOs can be permanently locked due to insufficient parameter validation in _grantWithCondition()

Lines of code ttps://github.com/code-423n4/2023-03-aragon/blob/4db573870aa4e1f40a3381cdd4ec006222e471fe/packages/contracts/src/core/permission/PermissionManager.solL312 Vulnerability details Impact It is possible to set the condition contract to an EOA when granting permission with a condition th...

Make proposals fail, no-revert on error, loss of funds

Lines of code Vulnerability details Impact The issue exists because anyone can call executein the multisig contract with will execute the proposal in the DAO contract. The problem here is that certain proposal will not revert when they fail because they have the uint256 allowFailureMap param set ...

createProposal snapshot block can temporarily desync with minApproval / minVotingPower

Lines of code Vulnerability details Impact minApproval and member list will be temporarily out of sync, potentially causing approval issues Proof of Concept uint64 snapshotBlock = block.number.toUint64 - 1; ... // Create the proposal Proposal storage proposal = proposalsproposalId;...

CounterV2.setNewVariable can be invoked by anyone.

Lines of code Vulnerability details Impact The CounterV2.setNewVariable is an open function which can be invoked by anyone. function setNewVariableuint256 newVariable external reinitializer2 newVariable = newVariable; This function is intended to be invoked by PluginSetupProcessor &...

No data validation, will result in all funds from the DAO contract stolen

Lines of code Vulnerability details Impact The vulnerability relies on the proposal creation function which forwards the proposal once accepted for the DAO contract to execute. Each proposal has a set of actions attached. struct Action address to; uint256 value; bytes data; function createProposa...

Prevent users from creating plugin repository proxy

Lines of code Vulnerability details Impact The attacker can Front-running the transaction and prevent users from creating plugin repository proxy. Proof of Concept The PluginRepoFactory.createPluginRepo is used to create a plugin repository proxy pointing to the pluginRepoBase implementation and...

Proposal may not be executed

Lines of code Vulnerability details Impact Proposal may not be executed Proof of Concept The protocol allows the member to have the voting power to create a proposal createProposal. Where the action struct in the parameter refers to be consumed by the DAO's execute function resulting in an extern...

AddresslistVoting.removeAddresses txn can be monitored to create proposal and cast vote.

Lines of code Vulnerability details Impact The AddresslistVoting contract contains removeAddresses function to remove the addresses from address list. function removeAddresses address calldata members external authUPDATEADDRESSESPERMISSIONID removeAddressesmembers; emit MembersRemovedmembers:...

Multisig: Users can approve proposals even after getting removed

Lines of code Vulnerability details Impact The Multisig contract intends to enable the creation and approval of proposals among a predetermined list of multisig addresses. The multisig addresses can be added or removed by a authorative identity. While creating a new proposal a snapshotBlock...

Locked Ether

Lines of code Vulnerability details Impact Contract with a payable function, but without a withdrawal capacity. Proof of Concept File: packages/contracts/src/core/dao/DAO.sol 233: external payable override 284: receive external payable Every Ether sent to DAO.sol will be lost. Tools Used VS Code...

Use of timestamp for comparisons

Lines of code Vulnerability details Impact The timestamp in use can be manipulated causing a logic bug in the checks performed Proof of Concept Tools Used slither Recommended Mitigation Steps use random number generation --- The text was updated successfully, but these errors were encountered: Al...

there is no way withdraw eth

Lines of code Vulnerability details Impact eth will be stuck inside the contract Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The text was update...

Everything wrong with deposit method

Lines of code Vulnerability details Impact No Accounting, Wrong data in Event emission in case of Fees on transfer Tokens and limited usecase. Proof of Concept File: DAO.sol function deposit address token, uint256 amount, string calldata reference external payable override if amount == 0 revert...

Funds in DAO contract cannot be withdrawn

Lines of code Vulnerability details Impact The DAO contract can receive funds but can not withdraw, Funds occasionally sent by users will be stuck in those contracts. Proof of Concept Receive funds through deposit function in DAO.solL218-L236 /// @inheritdoc IDAO function deposit address token,...

A malicious actor can create a non malicious plugin, but then simply change the implementation of the plugin repo, allowing him to change the logic and create malicious plugins

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Any developer can create their plugin repo and others can use them to download various plugins. The problem is that the plugin repo's are upgradeable, and the owner access and permissions to upgrade are...

INITIALIZE FUNCTION IN COUNTERV2.SOL CAN BE INVOKED MULTIPLE TIMES FROM THE IMPLEMENTATION CONTRACT

Lines of code Vulnerability details Impact Initialize function in CounterV2.sol can be invoked multiple times from the implementation contract. This means a compromised implementation can reinitialize the contract above. Usually in Upgradeable contract, an initialize function is protected by the...

User may force fail the action from the DAO:execute

Lines of code Vulnerability details Description The execute function from the DAO.sol contract allow to execution of any call to any address if the caller has appropriate permission. Some calls are expected to be always successfully executed, and some may revert and execute will continue the...

Malicious member can steal funds from the DAO contract.

Lines of code Vulnerability details Impact The malicious member can steal assets from DAO protocol. Proof of Concept The protocol allows the member to have the voting power to create a proposal createProposal. Where the action struct in the parameter refers to be consumed by the DAO's execute...