The DAO

The executeProposal function calls p.recipient.call.value(p.amount)(_transactionData) forwarding all gas before calling closeProposal. While p.proposalPassed is set before the call, closeProposal (which updates sumOfProposalDeposits and p.open) runs after, allowing a malicious recipient to re-enter and observe inconsistent state. This is the vulnerability exploited in the 2016 DAO hack.

An attacker can drain the entire ETH balance of the DAO through recursive calls during proposal execution.

Move closeProposal and all state updates before the external call, following the checks-effects-interactions pattern.

Token.transfer requires plutocracy.getOrModifyBlocked(_to) to return true for the transfer to succeed. However, getOrModifyBlocked returns true when an account IS blocked and false when NOT blocked. This means transfers only succeed TO blocked accounts and fail for unblocked recipients. The check should be on the sender (not recipient) and should prevent transfer when blocked (not require it).

The token transfer blocking mechanism is completely inverted. Blocked voters can freely transfer tokens since the sender is never checked. Normal transfers to non-voting accounts fail. This renders the vote-manipulation prevention useless and breaks normal token functionality.

Check the sender instead of recipient, and invert the condition to prevent transfer when the sender IS blocked.

In unVoteAll(), the call unVote(i) passes the loop index instead of the actual proposal ID votingRegister[msg.sender][i]. This causes unVote to operate on wrong proposals (0, 1, 2…) instead of the user’s actual voted proposals, corrupting vote tallies and enabling double-voting.

Vote tallies for unrelated proposals are corrupted (votes subtracted from wrong proposals). The user’s actual votes remain, and after votingRegister is wiped, they can vote again on the same proposals, doubling their voting weight.

Pass the actual proposal ID from the voting register instead of the loop index.

The minQuorum() function divides by actualBalance(). If actualBalance() returns 0 (when DAO balance equals sumOfProposalDeposits), this causes a revert, preventing any proposal from being executed.

An attacker can submit proposals whose deposits sum to the DAO’s balance, permanently blocking all proposal execution and freezing DAO governance.

Add a zero-balance check that returns totalSupply as the quorum requirement when actualBalance is zero.

The __callback function is callable by anyone, not just the Oraclize service. An attacker can call it with arbitrary oraclize IDs to trigger executeVote on proposal 0 (the default mapping value) or front-run legitimate callbacks.

An attacker can trigger premature or unauthorized vote execution in the DTHPool, potentially disrupting the delegate’s intended voting strategy.

Add access control to verify the caller is the Oraclize callback address.

The vote() function uses token.balanceOf(msg.sender) to determine voting weight at the time of the call. There is no snapshot or checkpoint mechanism (e.g., ERC20Votes / ERC20Snapshot) to record balances at proposal creation time. An attacker can acquire a large token position (via flash loan, borrowing, or market purchase), vote on a proposal, and then transfer or sell the tokens. The unVote() function compounds this issue: it also uses the current balanceOf() to subtract votes, meaning if a voter transfers tokens between voting and unvoting, the accounting becomes inconsistent (subtracting a different amount than was added).

An attacker with temporary access to a large token balance (e.g., via a flash loan on a DEX or lending protocol that lists the DAO token) could swing any governance vote in a single transaction. They could pass malicious proposals to drain the DAO’s entire ether balance. Additionally, the vote weight mismatch between vote() and unVote() can corrupt the yea/nay tallies—a voter who transfers tokens after voting and then unvotes will leave phantom votes in the tally.

Use a snapshot-based voting mechanism where voting power is determined by token balances at the block when the proposal was created (similar to ERC20Votes getPastVotes()). Alternatively, store the voter’s balance at vote time in the proposal struct and use that stored value for unvoting. Consider adding a time-lock between token acquisition and voting eligibility.

In sendValues(), when token creation fails, the fallback send to the owner is not checked: owner.send(this.balance). If the send fails, ether is permanently trapped with no recovery mechanism.

Users who fund the proxy contract after closing time or when token creation fails permanently lose their ether if the refund send fails.

Revert the transaction if the refund send fails.

The trusteeWithdraw() function performs arithmetic (this.balance + mainDAO.balanceOf(this)) - mainDAO.totalSupply() which can underflow if the sum is less than totalSupply. The send() return value is also unchecked.

An underflow would cause a massive erroneous send amount. If the send fails, trustee funds are stuck.

Add an underflow check and verify the send return value.

The unVoteAll() function iterates over votingRegister[msg.sender], an array that grows unboundedly each time vote() is called (line 208). Every call to vote() pushes a new entry via votingRegister[msg.sender].push(_proposalID) with no size limit. If a user votes on a large number of proposals, the unVoteAll() loop will eventually exceed the block gas limit, permanently bricking the function for that user. The code itself contains the comment // DANGEROUS loop with dynamic length - needs improvement.

A user who has voted on many proposals will be unable to call unVoteAll() as it will revert due to out-of-gas. Since unVoteAll() resets blocked[msg.sender] and removes votes, the user could become permanently blocked from transferring their DAO tokens. The commented-out withdraw() function also calls unVoteAll(), meaning any future withdrawal mechanism relying on it would also be bricked.

Implement a paginated unvote pattern that accepts a range of indices, or use a pull-over-push pattern where individual proposals can be unvoted one at a time. Alternatively, cap the number of active votes per user.

When a proposal reaches quorum, executeProposal attempts to refund the proposal deposit to p.creator using .send() and reverts the entire transaction if the send fails. If the proposal creator is a smart contract that reverts on receiving ETH (e.g., has no payable fallback, or intentionally reverts), the deposit refund fails, causing the entire executeProposal call to revert. This permanently blocks execution of that proposal, even though it legitimately passed the vote. The proposal remains open, blocking voters’ tokens (via the blocked mapping) until executeProposalPeriod expires and someone calls executeProposal again to trigger closeProposal. Note that in the !allowedRecipients branch, the return value of p.creator.send() is intentionally NOT checked (the comment acknowledges this), but in the quorum-met branch the throw on failure creates the DoS vector.

A malicious actor can create a proposal from a contract that refuses ETH, gather enough community votes to reach quorum, and then the proposal can never execute. This wastes community voting resources, blocks voters’ tokens for the executeProposalPeriod duration, and prevents legitimate proposals from being carried out. The proposal deposit is also locked until the proposal is eventually closed.

Use a pull-over-push pattern for deposit refunds. Instead of sending the deposit inline and reverting on failure, credit the deposit to the creator’s balance in a mapping and let them withdraw it separately. Alternatively, remove the throw on send failure (as was already done in the !allowedRecipients branch) so that a failed refund does not block proposal execution.