Lobbyist Protocol

Smart Contract Audit Report

Audit Summary

Lobbyist Protocol is building a new platform where users can create proposals and earn rewards.

For this audit, we reviewed the following Pool contracts at:

• 0xD72BBE31925Fa84a66C46A23D52fCFe8b2F8840a on the Ethereum Mainnet.
• 0x691aa8afa4eb9bf2537ef54ded660129dc35c6e3 on the Optimism Mainnet.
• 0x691Aa8AfA4eB9bF2537ef54dEd660129DC35C6e3 on the Binance Smart Chain Mainnet.
• 0xcfB7c77DCe535C7d34C5b13e813f2b7060011596 on the Polygon Mainnet.
• 0xdf5b6975c9f833dd73b327c675458f420998174f on the Fantom Mainnet.
• 0x691aa8afa4eb9bf2537ef54ded660129dc35c6e3 on the Arbitrum Mainnet.
• 0x7d3641d200c82a4ec599f33ff9392a482a878e79 on the Avalanche Mainnet.

Audit Findings

All findings have been resolved, though some centralized aspects are present.
Date: October 18th, 2022.
Updated: October 27th, 2022 to reflect the Mainnet addresses of each Pool contract.

Finding #1 - Pool - High (Resolved)

Description: Although a require-statement exists in the createPool() function to check that the proposal ID has not already been used, the isCreated mapping is never updated for a newly created proposal ID.
Risk/Impact: A proposal ID can be used more than once when creating a pool.
Recommendation: The team should set the isCreated mapping for a newly created proposal ID to true directly after the first require-statement (to follow the Checks-Effects-Interactions pattern) in the createPool() function as follows:

function createPool(PoolData memory _pooldata) external payable {
	require(!isCreated[_pooldata.proposalId], "pool already created for proposal");
	isCreated[_pooldata.proposalId] = true;
...
IERC20(_pooldata.rewardCurrency).transferFrom( 
msg.sender,
address(this),
_pooldata.rewardAmount
);

Resolution: The team has implemented the above recommendation.

---

Finding #2 - Pool - High (Resolved)

Description: In the addReward() function, although the user specifies the ID of the pool to add reward tokens to, the rewardAmount value is always increased for the last created pool.

function addReward(uint256 id, uint256 amount) external {
	require(poolCount > id, "invalid id");
	IERC20(poolDatas[id].rewardCurrency).transferFrom(
		msg.sender,
		address(this),
		amount
	);
...
	poolDatas[poolCount].rewardAmount += amount;
}

Risk/Impact: The tokens will always be allocated to the last created pool rather than the specified pool ID.
Recommendation: The rewardAmount value should be increased for the pool ID that is passed into the addReward() function.
Resolution: The team has implemented the above recommendation.

---

Finding #3 - Pool - High (Resolved)

Description: Although the createPool() function is payable, the ETH transferred to the Admin wallet is supplied from the contract and msg.value is never used.

function createPool(PoolData memory _pooldata) external payable {
...

	payable(admin).transfer(0.01 * 1e18);

Risk/Impact: The user does not have to supply any ETH to create a pool, and the ETH transferred to the Admin wallet is instead supplied from the contract.
Recommendation: The function should be modified to enforce that the user has supplied a msg.value of 0.01 ether and the msg.value is sent to the Admin wallet.
Resolution: The team has implemented the above recommendation.

---

Finding #4 - Pool - High (Resolved)

Description: In the addReward() function, the fee is not deducted from rewardAmount before it is allocated to a pool.

function addReward(uint256 id, uint256 amount) external {
	require(poolCount > id, "invalid id");
	IERC20(poolDatas[id].rewardCurrency).transferFrom(
		msg.sender,
		address(this),
		amount
	);
	IERC20(poolDatas[id].rewardCurrency).transfer(
		owner(),
		(amount * fee) / 1000000
	);
	poolDatas[poolCount].rewardAmount += amount;
}

Risk/Impact: There will be a mismatch between the number of tokens in the contract and the rewardAmount allocated to a pool as the fee is not accounted for.
Recommendation: The fee should be deducted from the amount value before increasing the rewardAmount of a pool.
Resolution: The team has implemented the above recommendation.

---

Finding #5 - Pool - High (Resolved)

Description: Any user can create a new pool and set the reward token as any token address in the createPool() function, however the logic in the contract is not suitable for fee-on-transfer tokens.
Risk/Impact: If a fee-on-transfer token is set as a reward token, a mismatch will occur between the number of tokens sent to the contract and the rewardAmount associated with the pool.
Recommendation: The team should add support for fee-on-transfer tokens in the createPool() and addReward() functions by using the contract balance before and after transferring tokens to determine how many tokens should be assigned to rewardAmount.
Resolution: The team has modified the contract to ensure only the Admin can pre-approve reward token addresses. The team does not intend on approving any fee-on-transfer tokens.

---

Finding #6 - Pool - High (Resolved)

Description: Any user can create a new pool and set the reward token as any token address in the createPool() function, however the logic in the contract is not suitable for ERC-777-compliant tokens.
Risk/Impact: Any voter can leverage the fallback function called on each ERC-777 token transfer to perform a reentrancy attack on the contract's claim() function and drain the contract's balance.
Recommendation: The team should restructure the logic in the contract to comply with the Checks-Effects-Interactions pattern or utilize the nonReentrant() modifier in the ReentrancyGuard contract to prevent any reentrancy attacks.
Resolution: The team has implemented the above recommendation.

---

Finding #7 - Pool - High (Resolved)

Description: The owner can withdraw any reward tokens from the contract at any time by use of the withdrawAll() and withdrawTokens() functions.
Risk/Impact: The team can remove any tokens that were sent to the contract to be used as rewards.
Recommendation: The team should ensure that tokens that have been sent to the contract as rewards cannot be withdrawn.
Resolution: The team has removed the above functions from the contract.

---

Finding #8 - Pool - Low (Resolved)

Description: The owner can use the lock() function to temporarily set ownership to address(0). Ownership is restored after the duration of time determined by the owner has passed and the unlock() function can be used.
Risk/Impact: The unlock() function has the potential to be used after ownership has been set to address(0), which will restore ownership to the original owner that initially created the ownership lock. This can be used in a nefarious way by the project team to restore ownership and change fee percentages.
Recommendation: The unlock() function should be modified to set the _previousOwner equal to address(0) at the end of the unlock() function to prevent it from being used more than once per lock.
Resolution: The team has removed the lock() and unlock() functions from the contract.

Contract Overview

• Any user can create a new pool by providing an ID, name, description, platform type, outcome, reward token, and reward amount.
• The reward token used must have been pre-approved by the Admin.
• The reward amount will be transferred from the user to the contract in the form of the reward token.
• Any user can add reward tokens to a created pool at any time.
• A reward fee (set by the owner) is deducted from the reward amount and is allocated to the owner on all pool creations and reward adds.
• The team must exercise caution when assigning reward tokens to avoid using a fee-on-transfer token. If a fee-on-transfer token is assigned, the team must ensure that the proper addresses are excluded from fees.
• The Admin can close an open pool at any time by specifying a number of voter addresses and their number of votes.
• The platform's voting functionality does not take place in the Pool contract and is therefore not in scope for this audit.
• A number of tokens will be allocated to each voter based on their number of votes and the total calculated reward per vote value.
• Voters can claim the rewards due to them at any time.
• If the Admin specified zero votes associated with a pool, the full number of reward tokens are transferred to the creator of the pool.
• The Admin can add/remove any address as a valid reward token at any time.
• The owner can set the reward fee to any percentage at any time.

Audit Results

Inheritance Chart

Function Graph

Functions Overview

 ($) = payable function
 # = non-constant function
 
 Int = Internal
 Ext = External
 Pub = Public

 + [Int] IERC20 
    - [Ext] totalSupply
    - [Ext] balanceOf
    - [Ext] decimals
    - [Ext] transfer #
    - [Ext] allowance
    - [Ext] approve #
    - [Ext] transferFrom #

 +  Context 
    - [Int] _msgSender
    - [Int] _msgData

 +  Ownable (Context)
    - [Pub]  #
    - [Pub] owner
    - [Pub] renounceOwnership #
       - modifiers: onlyOwner
    - [Pub] transferOwnership #
       - modifiers: onlyOwner
	- [Pub] geUnlockTime

 +  ReentrancyGuard 
    - [Pub]  #

 +  Pool (Ownable, ReentrancyGuard)
    - [Pub]  #
    - [Ext] setAdminSetting #
       - modifiers: onlyOwner
    - [Ext] createPool ($)
    - [Ext] addReward #
    - [Ext] closePool #
       - modifiers: onlyAdmin
    - [Ext] claim #
       - modifiers: nonReentrant
    - [Ext] setCurrency #
       - modifiers: onlyAdmin

About SourceHat

SourceHat has quickly grown to have one of the most experienced and well-equipped smart contract auditing teams in the industry. Our team has conducted 1800+ solidity smart contract audits covering all major project types and protocols, securing a total of over $50 billion U.S. dollars in on-chain value!
Our firm is well-reputed in the community and is trusted as a top smart contract auditing company for the review of solidity code, no matter how complex. Our team of experienced solidity smart contract auditors performs audits for tokens, NFTs, crowdsales, marketplaces, gambling games, financial protocols, and more!

Contact us today to get a free quote for a smart contract audit of your project!

What is a SourceHat Audit?

Typically, a smart contract audit is a comprehensive review process designed to discover logical errors, security vulnerabilities, and optimization opportunities within code. A SourceHat Audit takes this a step further by verifying economic logic to ensure the stability of smart contracts and highlighting privileged functionality to create a report that is easy to understand for developers and community members alike.

How Do I Interpret the Findings?

Each of our Findings will be labeled with a Severity level. We always recommend the team resolve High, Medium, and Low severity findings prior to deploying the code to the mainnet. Here is a breakdown on what each Severity level means for the project:

• High severity indicates that the issue puts a large number of users' funds at risk and has a high probability of exploitation, or the smart contract contains serious logical issues which can prevent the code from operating as intended.
• Medium severity issues are those which place at least some users' funds at risk and has a medium to high probability of exploitation.
• Low severity issues have a relatively minor risk association; these issues have a low probability of occurring or may have a minimal impact.
• Informational issues pose no immediate risk, but inform the project team of opportunities for gas optimizations and following smart contract security best practices.

• Go Home