0% found this document useful (0 votes)
86 views20 pages

PeckShield Audit Report Battle Stakes v1.0

The document is a smart contract audit report for the Battle Stakes protocol conducted by PeckShield. It finds several issues in the initial smart contracts related to security and performance and provides recommendations for improvement. The key findings include issues with the logic for selling arenas, accommodating non-ERC20 tokens, compatibility with deflationary tokens, and potential trust issues with admin keys. Overall, the audit finds room for enhancing the smart contracts.

Uploaded by

Shop Tok
Copyright
© © All Rights Reserved
We take content rights seriously. If you suspect this is your content, claim it here.
Available Formats
Download as PDF, TXT or read online on Scribd
0% found this document useful (0 votes)
86 views20 pages

PeckShield Audit Report Battle Stakes v1.0

The document is a smart contract audit report for the Battle Stakes protocol conducted by PeckShield. It finds several issues in the initial smart contracts related to security and performance and provides recommendations for improvement. The key findings include issues with the logic for selling arenas, accommodating non-ERC20 tokens, compatibility with deflationary tokens, and potential trust issues with admin keys. Overall, the audit finds room for enhancing the smart contracts.

Uploaded by

Shop Tok
Copyright
© © All Rights Reserved
We take content rights seriously. If you suspect this is your content, claim it here.
Available Formats
Download as PDF, TXT or read online on Scribd
You are on page 1/ 20

Public

SMART CONTRACT AUDIT REPORT


for

InvoBlox Battle Stakes

Prepared By: Xiaomi Huang

PeckShield
October 29, 2022

1/20 PeckShield Audit Report #: 2022-369


Public

Document Properties

Client InvoBlox
Title Smart Contract Audit Report
Target Battle Stakes
Version 1.0
Author Stephen Bie
Auditors Stephen Bie, Xuxian Jiang
Reviewed by Xiaomi Huang
Approved by Xuxian Jiang
Classification Public

Version Info

Version Date Author(s) Description


1.0 October 29, 2022 Stephen Bie Final Release
1.0-rc October 24, 2022 Stephen Bie Release Candidate

Contact

For more information about this document and its contents, please contact PeckShield Inc.

Name Xiaomi Huang


Phone +86 183 5897 7782
Email contact@peckshield.com

2/20 PeckShield Audit Report #: 2022-369


Public

Contents

1 Introduction 4
1.1 About Battle Stakes . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 4
1.2 About PeckShield . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 5
1.3 Methodology . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 5
1.4 Disclaimer . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 7

2 Findings 9
2.1 Summary . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 9
2.2 Key Findings . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 10

3 Detailed Results 11
3.1 Revisited Logic of StakingContract::sellAreenaByGen() . . . . . . . . . . . . . . . . 11
3.2 Accommodation of Non-ERC20-Compliant Tokens . . . . . . . . . . . . . . . . . . . 12
3.3 Incompatibility with Deflationary/Rebasing Tokens . . . . . . . . . . . . . . . . . . . 14
3.4 Improved Logic of StakingContract::CreateBattle() . . . . . . . . . . . . . . . . . . 16
3.5 Trust Issue of Admin Keys . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 17

4 Conclusion 19

References 20

3/20 PeckShield Audit Report #: 2022-369


Public

1 | Introduction

Given the opportunity to review the design document and related smart contract source code of the
Battle Stakes protocol, we outline in the report our systematic approach to evaluate potential security
issues in the smart contract implementation, expose possible semantic inconsistencies between smart
contract code and design document, and provide additional suggestions or recommendations for
improvement. Our results show that the given version of smart contracts can be further improved
due to the presence of several issues related to either security or performance. This document outlines
our audit results.

1.1 About Battle Stakes


Battle Stakes is an innovative DeFi protocol that is designed to be a hybrid of staking and play-to-
earn project. The protocol enables users to earn BUSD, GEN (a native coin to the protocol), and ARENA
(a token designed to increase in value as the Battle Stakes network grows). It provides a new way to
do yield farming by introducing wallet against wallet staking. The basic information of the audited
protocol is as follows:

Table 1.1: Basic Information of Battle Stakes

Item Description
Target Battle Stakes
Type EVM Smart Contract
Language Solidity
Audit Method Whitebox
Latest Audit Report October 29, 2022

In the following, we show the Git repository of reviewed files and the commit hash value used in
this audit.

• https://gitlab.invozone.com/invoblox/stake-game-dapp/smart-contracts.git (e40bf7f)

4/20 PeckShield Audit Report #: 2022-369


Public

And this is the commit ID after all fixes for the issues found in the audit have been checked in:

• https://gitlab.invozone.com/invoblox/stake-game-dapp/smart-contracts.git (894e45a)

1.2 About PeckShield


PeckShield Inc. [7] is a leading blockchain security company with the goal of elevating the secu-
rity, privacy, and usability of current blockchain ecosystems by offering top-notch, industry-leading
services and products (including the service of smart contract auditing). We are reachable at Telegram
(https://t.me/peckshield), Twitter (http://twitter.com/peckshield), or Email (contact@peckshield.com).

Table 1.2: Vulnerability Severity Classification

High Critical High Medium


Impact

Medium High Medium Low

Low Medium Low Low

High Medium Low

Likelihood

1.3 Methodology
To standardize the evaluation, we define the following terminology based on OWASP Risk Rating
Methodology [6]:

• Likelihood represents how likely a particular vulnerability is to be uncovered and exploited in


the wild;

• Impact measures the technical loss and business damage of a successful attack;

• Severity demonstrates the overall criticality of the risk.

Likelihood and impact are categorized into three ratings: H, M and L, i.e., high, medium and
low respectively. Severity is determined by likelihood and impact and can be classified into four
categories accordingly, i.e., Critical, High, Medium, Low shown in Table 1.2.
To evaluate the risk, we go through a list of check items and each would be labeled with
a severity category. For one check item, if our tool or analysis does not identify any issue, the

5/20 PeckShield Audit Report #: 2022-369


Public

Table 1.3: The Full List of Check Items

Category Check Item


Constructor Mismatch
Ownership Takeover
Redundant Fallback Function
Overflows & Underflows
Reentrancy
Money-Giving Bug
Blackhole
Unauthorized Self-Destruct
Revert DoS
Basic Coding Bugs
Unchecked External Call
Gasless Send
Send Instead Of Transfer
Costly Loop
(Unsafe) Use Of Untrusted Libraries
(Unsafe) Use Of Predictable Variables
Transaction Ordering Dependence
Deprecated Uses
Semantic Consistency Checks Semantic Consistency Checks
Business Logics Review
Functionality Checks
Authentication Management
Access Control & Authorization
Oracle Security
Digital Asset Escrow
Advanced DeFi Scrutiny
Kill-Switch Mechanism
Operation Trails & Event Generation
ERC20 Idiosyncrasies Handling
Frontend-Contract Integration
Deployment Consistency
Holistic Risk Management
Avoiding Use of Variadic Byte Array
Using Fixed Compiler Version
Additional Recommendations Making Visibility Level Explicit
Making Type Inference Explicit
Adhering To Function Declaration Strictly
Following Other Best Practices

6/20 PeckShield Audit Report #: 2022-369


Public

contract is considered safe regarding the check item. For any discovered issue, we might further
deploy contracts on our private testnet and run tests to confirm the findings. If necessary, we would
additionally build a PoC to demonstrate the possibility of exploitation. The concrete list of check
items is shown in Table 1.3.
In particular, we perform the audit according to the following procedure:

• Basic Coding Bugs: We first statically analyze given smart contracts with our proprietary static
code analyzer for known coding bugs, and then manually verify (reject or confirm) all the issues
found by our tool.

• Semantic Consistency Checks: We then manually check the logic of implemented smart con-
tracts and compare with the description in the white paper.

• Advanced DeFi Scrutiny: We further review business logics, examine system operations, and
place DeFi-related aspects under scrutiny to uncover possible pitfalls and/or bugs.

• Additional Recommendations: We also provide additional suggestions regarding the coding and
development of smart contracts from the perspective of proven programming practices.

To better describe each issue we identified, we categorize the findings with Common Weakness
Enumeration (CWE-699) [5], which is a community-developed list of software weakness types to
better delineate and organize weaknesses around concepts frequently encountered in software devel-
opment. Though some categories used in CWE-699 may not be relevant in smart contracts, we use
the CWE categories in Table 1.4 to classify our findings.

1.4 Disclaimer
Note that this security audit is not designed to replace functional tests required before any software
release, and does not give any warranties on finding all possible security issues of the given smart
contract(s) or blockchain software, i.e., the evaluation result does not guarantee the nonexistence
of any further findings of security issues. As one audit-based assessment cannot be considered
comprehensive, we always recommend proceeding with several independent audits and a public bug
bounty program to ensure the security of smart contract(s). Last but not least, this security audit
should not be used as investment advice.

7/20 PeckShield Audit Report #: 2022-369


Public

Table 1.4: Common Weakness Enumeration (CWE) Classifications Used in This Audit

Category Summary
Configuration Weaknesses in this category are typically introduced during
the configuration of the software.
Data Processing Issues Weaknesses in this category are typically found in functional-
ity that processes data.
Numeric Errors Weaknesses in this category are related to improper calcula-
tion or conversion of numbers.
Security Features Weaknesses in this category are concerned with topics like
authentication, access control, confidentiality, cryptography,
and privilege management. (Software security is not security
software.)
Time and State Weaknesses in this category are related to the improper man-
agement of time and state in an environment that supports
simultaneous or near-simultaneous computation by multiple
systems, processes, or threads.
Error Conditions, Weaknesses in this category include weaknesses that occur if
Return Values, a function does not generate the correct return/status code,
Status Codes or if the application does not handle all possible return/status
codes that could be generated by a function.
Resource Management Weaknesses in this category are related to improper manage-
ment of system resources.
Behavioral Issues Weaknesses in this category are related to unexpected behav-
iors from code that an application uses.
Business Logics Weaknesses in this category identify some of the underlying
problems that commonly allow attackers to manipulate the
business logic of an application. Errors in business logic can
be devastating to an entire application.
Initialization and Cleanup Weaknesses in this category occur in behaviors that are used
for initialization and breakdown.
Arguments and Parameters Weaknesses in this category are related to improper use of
arguments or parameters within function calls.
Expression Issues Weaknesses in this category are related to incorrectly written
expressions within code.
Coding Practices Weaknesses in this category are related to coding practices
that are deemed unsafe and increase the chances that an ex-
ploitable vulnerability will be present in the application. They
may not directly introduce a vulnerability, but indicate the
product has not been carefully developed or maintained.

8/20 PeckShield Audit Report #: 2022-369


Public

2 | Findings

2.1 Summary
Here is a summary of our findings after analyzing the Battle Stakes implementation. During the
first phase of our audit, we study the smart contract source code and run our in-house static code
analyzer through the codebase. The purpose here is to statically identify known coding bugs, and
then manually verify (reject or confirm) issues reported by our tool. We further manually review
business logic, examine system operations, and place DeFi-related aspects under scrutiny to uncover
possible pitfalls and/or bugs.

Severity # of Findings
Critical 1
High 0
Medium 2
Low 2
Informational 0
Total 5

We have so far identified a list of potential issues: some of them involve subtle corner cases
that might not be previously thought of, while others refer to unusual interactions among multiple
contracts. For each uncovered issue, we have therefore developed test cases for reasoning, reproduc-
tion, and/or verification. After further analysis and internal discussion, we determined a few issues
of varying severities that need to be brought up and paid more attention to, which are categorized in
the above table. More information can be found in the next subsection, and the detailed discussions
of each of them are in Section 3.

9/20 PeckShield Audit Report #: 2022-369


Public

2.2 Key Findings


Overall, these smart contracts are well-designed and engineered, though the implementation can
be improved by resolving the identified issues (shown in Table 2.1), including 1 critical-severity
vulnerability, 2 medium-severity vulnerabilities, and 2 low-severity vulnerabilities.

Table 2.1: Key Battle Stakes Audit Findings

ID Severity Title Category Status


PVE-001 Critical Revisited Logic of StakingCon- Business Logic Mitigated
tract::sellAreenaByGen()
PVE-002 Low Accommodation of Non-ERC20- Coding Practices Confirmed
Compliant Tokens
PVE-003 Low Incompatibility with Deflationary/Re- Business Logic Confirmed
basing Tokens
PVE-004 Medium Improved Logic of StakingCon- Business Logic Fixed
tract::CreateBattle()
PVE-005 Medium Trust Issue of Admin Keys Security Features Confirmed

Beside the identified issues, we emphasize that for any user-facing applications and services, it is
always important to develop necessary risk-control mechanisms and make contingency plans, which
may need to be exercised before the mainnet deployment. The risk-control mechanisms should kick
in at the very moment when the contracts are being deployed on mainnet. Please refer to Section 3
for details.

10/20 PeckShield Audit Report #: 2022-369


Public

3 | Detailed Results

3.1 Revisited Logic of StakingContract::sellAreenaByGen()

• ID: PVE-001 • Target: StakingContract


• Severity: Critical • Category: Business Logic [4]
• Likelihood: High • CWE subcategory: CWE-841 [2]
• Impact: High

Description
In the Battle Stakes protocol, the StakingContract contract is one of the main entries for interaction
with users. In particular, one entry routine, sellAreenaByGen(), is designed to sell the ARENA token and
get in return the GEN token. While examining its logic, we notice there is an improper implementation
that needs to be improved.
To elaborate, we show below the related code snippet of the StakingContract contract. Inside
the sellAreenaByGen() routine, the internal calculateTotalgenthroughAreena() routine is called (line
745) to calculate the exchanged amount of the GEN token for the given ARENA token. After further
analysis, we observe the user input _genPrice is directly used in the calculateTotalgenthroughAreena()
routine (line 753) to calculate the exchanged amount of the GEN token, which directly undermines
the assumption of the design.
738 function s e ll Ar e en a By Ge n ( uint256 _areenaAmount , uint256 _genPrice ) external {
739
740 ...
741
742 // uint256 a r e e n a P r i c e I n B u s d = c a l c u l a t e A r e e n a P r i c e F o r G e n ( _areenaAmount ) ;
743 // uint256 _ t o t a l A m o u n t O f G e n = ( a r e e n a P r i c e I n B u s d . mul (1 e18 ) ) . div ( _genPrice ) ;
744 ( uint256 areenaPriceInBusd , uint256 _ t o t a l A m o u n t O f G e n ) =
745 c a l c u l a t e T o t a l g e n t h r o u g h A r e e n a ( _areenaAmount , _genPrice ) ;
746
747 ...
748 }
749

11/20 PeckShield Audit Report #: 2022-369


Public

750 function c a l c u l a t e T o t a l g e n t h r o u g h A r e e n a ( uint256 _areenaAmount , uint256 _genPrice )


public view returns ( uint256 areenaPriceInBusd , uint256 _ t o t a l A m o u n t O f G e n ) {
751
752 a r e e n a P r i c e I n B u s d = c a l c u l a t e A r e e n a P r i c e F o r G e n ( _areenaAmount ) ;
753 _ t o t a l A m o u n t O f G e n = ( a r e e n a P r i c e I n B u s d . mul (1 e18 ) ) . div ( _genPrice ) ;
754
755 return ( areenaPriceInBusd , _ t o t a l A m o u n t O f G e n ) ;
756 }

Listing 3.1: StakingContract::sellAreenaByGen()

Note another routine, i.e., sellGenThroughDashboard(), shares the same issue.

Recommendation The GEN token price should not be specified by users.


Status The issue has been mitigated in the following commits: 8ca3c4a and 894e45a.

3.2 Accommodation of Non-ERC20-Compliant Tokens

• ID: PVE-002 • Target: Multiple Contracts


• Severity: Low • Category: Business Logic [4]
• Likelihood: Low • CWE subcategory: CWE-841 [2]
• Impact: Low

Description
Though there is a standardized ERC-20 specification, many token contracts may not strictly follow the
specification or have additional functionalities beyond the specification. In this section, we examine
the transferFrom() routine and possible idiosyncrasies from current widely-used token contracts.
In particular, we use the popular stablecoin, i.e., USDT, as our example. We show the related
code snippet below. Specifically, the transferFrom() routine does not have a return value defined
and implemented. However, the IERC20 interface has defined the transferFrom() interface with a bool
return value. As a result, the call to transferFrom() may expect a return value. With the lack of
return value of USDT’s transferFrom(), the call will be unfortunately reverted.
171 function transferFrom ( address _from , address _to , uint _value ) public
o nl yP a yl oa d Si z e (3 * 32) {
172 var _allowance = allowed [ _from ][ msg . sender ];
173
174 // Check is not needed because sub ( _allowance , _value ) will already throw if
this condition is not met
175 // if ( _value > _allowance ) throw ;
176
177 uint fee = ( _value . mul ( ba s is Po i nt sR a te ) ) . div (10000) ;
178 if ( fee > maximumFee ) {

12/20 PeckShield Audit Report #: 2022-369


Public

179 fee = maximumFee ;


180 }
181 if ( _allowance < MAX_UINT ) {
182 allowed [ _from ][ msg . sender ] = _allowance . sub ( _value ) ;
183 }
184 uint sendAmount = _value . sub ( fee ) ;
185 balances [ _from ] = balances [ _from ]. sub ( _value ) ;
186 balances [ _to ] = balances [ _to ]. add ( sendAmount ) ;
187 if ( fee > 0) {
188 balances [ owner ] = balances [ owner ]. add ( fee ) ;
189 Transfer ( _from , owner , fee ) ;
190 }
191 Transfer ( _from , _to , sendAmount ) ;
192 }

Listing 3.2: USDT Token Contract

Because of that, a normal call to transferFrom() is suggested to use the safe version, i.e.,
safeTransferFrom(). In essence, it is a wrapper around ERC20 operations that may either throw
on failure or return false without reverts. Moreover, the safe version also supports tokens that return
no value (and instead revert or throw on failure). Note that non-reverting calls are assumed to be
successful. Similarly, there is a safe version of transfer() as well, i.e., safeTransfer().
In the following, we show the CreateBattle() routine in the StakingContract contract. If the USDT
token is supported as genToken, the unsafe version of genToken.transferFrom(battle.creator, address
(this), amountFromUser) (line 180) may revert as there is no return value in the USDT token contract’s
transferFrom() implementation (but the IERC20 interface expects a return value).
174 function CreateBattle ( uint256 _amount , uint256 _r is k Pe rc e nt ag e ) external {
175 ...
176 if ( genToken . balanceOf ( battle . creator ) < stakeAmount ) {
177
178 uint256 am ountFr omUser = genToken . balanceOf ( battle . creator ) ;
179
180 genToken . transferFrom ( battle . creator , address ( this ) , amo untFro mUser ) ;
181
182 uint256 a m o u n t F r o m A d d r e s s = stakeAmount - amo untFro mUser ;
183 player . g e n A m o u n t P l u s B o n u s -= a m o u n t F r o m A d d r e s s ;
184
185 } else {
186 genToken . transferFrom ( battle . creator , address ( this ) , stakeAmount ) ;
187 }
188
189 ...
190 }

Listing 3.3: StakingContract::CreateBattle()

Recommendation Accommodate the above-mentioned idiosyncrasy about ERC20-related


transfer()/transferFrom().

13/20 PeckShield Audit Report #: 2022-369


Public

Status The issue has been confirmed by the team. There is no need to support Non-ERC20-
Compliant tokens.

3.3 Incompatibility with Deflationary/Rebasing Tokens

• ID: PVE-003 • Target: StakingContract


• Severity: Low • Category: Business Logic [4]
• Likelihood: Low • CWE subcategory: CWE-841 [2]
• Impact: Low

Description
In the Battle Stakes implementation, the StakingContract contract is one of the main entries for
interaction with users. In particular, one entry routine, i.e., CreateBattle(), is used by the user to
create a new staking battle. Naturally, the contract implements a number of low-level helper routines
to transfer assets in or out of the StakingContract contract. These asset-transferring routines work
as expected with standard ERC20 tokens: namely the vault’s internal asset balances are always
consistent with actual token balances maintained in individual ERC20 token contracts.
174 function CreateBattle ( uint256 _amount , uint256 _r is k Pe rc e nt ag e ) external {
175 ...
176 if ( genToken . balanceOf ( battle . creator ) < stakeAmount ) {
177
178 uint256 am ountFr omUser = genToken . balanceOf ( battle . creator ) ;
179
180 genToken . transferFrom ( battle . creator , address ( this ) , amo untFro mUser ) ;
181
182 uint256 a m o u n t F r o m A d d r e s s = stakeAmount - amo untFro mUser ;
183 player . g e n A m o u n t P l u s B o n u s -= a m o u n t F r o m A d d r e s s ;
184
185 } else {
186 genToken . transferFrom ( battle . creator , address ( this ) , stakeAmount ) ;
187 }
188
189 emit createBattle ( battle . creator , stakeAmount , battleId ) ;
190
191 referalPerson [ battleId ]. c r e a t o r R e f e r a l P e r s o n = r e f e r a l P e r s o n A d d r e s s [ msg . sender ];
192 referalPerson [ battleId ]. battleCreator = battle . creator ;
193 p la ye r Ba tt l eI d s [ battle . creator ]. push ( battleId ) ;
194
195 battleId ++;
196 battle . stakeAmount = stakeAmount ;
197 battle . riskP ercent age = _r i sk Pe r ce nt a ge ;
198 battle . c r e a t o r S t a r t i n g t i m e = block . timestamp ;

14/20 PeckShield Audit Report #: 2022-369


Public

199 }

Listing 3.4: StakingContract::CreateBattle()

However, there exist other ERC20 tokens that may make certain customizations to their ERC20
contracts. One type of these tokens is deflationary tokens that charge certain fee for every transfer()
or transferFrom(). (Another type is rebasing tokens such as YAM.) As a result, this may not meet the
assumption behind these low-level asset-transferring routines. In other words, the above operations,
such as CreateBattle(), may introduce unexpected balance inconsistencies when comparing internal
asset records with external ERC20 token contracts. Apparently, these balance inconsistencies are
damaging to accurate and precise portfolio management of Battle Stakes and affects protocol-wide
operation and maintenance.
One possible mitigation is to measure the asset change right before and after the asset-transferring
routines. In other words, instead of bluntly assuming the amount parameter in transfer() or
transferFrom() will always result in full transfer, we need to ensure the increased or decreased amount
in the contract before and after the transfer() or transferFrom() is expected and aligned well with our
operation. Though these additional checks cost additional gas usage, we consider they are necessary
to deal with deflationary tokens or other customized ones if their support is deemed necessary.

Recommendation If current codebase needs to support possible deflationary tokens, it is better


to check the balance before and after the transfer()/transferFrom() call to ensure the book-keeping
amount is accurate. This support may bring additional gas cost. Also, keep in mind that certain
tokens may not be deflationary for the time being. However, they could have a control switch that
can be exercised to turn them into deflationary tokens. One example is the widely-adopted USDT.

Status The issue has been confirmed by the team.

15/20 PeckShield Audit Report #: 2022-369


Public

3.4 Improved Logic of StakingContract::CreateBattle()

• ID: PVE-004 • Target: StakingContract


• Severity: Medium • Category: Business Logic [4]
• Likelihood: High • CWE subcategory: CWE-841 [2]
• Impact: Low

Description
By design, the StakingContract contract implements a so-called staking battle mechanism, which
allows the user to create a new staking battle via CreateBattle() or join an active staking battle via
JoinBattle(). For both sides, whoever leaves first loses the staking battle. While examining the logic
of the CreateBattle() routine, we notice its current implementation should be improved.
To elaborate, we show below the related code snippet of the StakingContract contract. Inside the
CreateBattle() routine, the requirement of require(stakeAmount >= minimumStake, "You must stake
atleast 1 Gen tokens to enter into the battle.") is called (line 108) to ensure that the staking
amount of the new staking battle should be larger than or equal to the predefined minimumStake.
However, after further analysis, we notice the local stakeAmount variable carries decimals (line 103) but
the predefined minimumStake doesn’t (line 91). Given this, we suggest to improve the implementation
as below: uint256 private minimumStake = 1 * 1e18 (line 91).
91 uint256 private minimumStake = 1;
92
93 ...
94
95 function CreateBattle ( uint256 _amount , uint256 _r is k Pe rc e nt ag e ) external {
96
97 Player storage player = players [ msg . sender ];
98
99 Battle storage battle = battles [ battleId ];
100 battle . creator = msg . sender ;
101
102 uint256 stakeAmount = checkOption ( _amount ) ;
103 stakeAmount = stakeAmount . mul (1 e18 ) ;
104
105 require ( loginFromSite [ msg . sender ] == true ,
106 " You must have to login from site to enter into the battle . " ) ;
107
108 require ( stakeAmount >= minimumStake ,
109 " You must stake atleast 1 Gen tokens to enter into the battle . " ) ;
110 require ( stakeAmount <= ( stakeOptions [4]. mul (1 e18 ) ) ,
111 " You can not stake more then 1000 Gen tokens to create a battle . " ) ;
112
113 ...

16/20 PeckShield Audit Report #: 2022-369


Public

114 }

Listing 3.5: StakingContract::CreateBattle()

Recommendation Improve the above CreateBattle() by using the consistent decimals.

Status The issue has been addressed in the following commit: d0206d2.

3.5 Trust Issue of Admin Keys

• ID: PVE-005 • Target: Multiple Contracts


• Severity: Medium • Category: Security Features [3]
• Likelihood: Medium
• CWE subcategory: CWE-287 [1]
• Impact: Medium
Description
In the Battle Stakes protocol, there is a privileged owner account that plays a critical role in governing
and regulating the protocol-wide operations (e.g., configuring various system parameters). In the
following, we show the representative functions potentially affected by the privilege of the owner
account.
1025 function s e t T r e a s u r y W a l l e t ( address _walle tAddre ss ) external onlyOwner {
1026 tre asuryW allet = _ wallet Addres s ;
1027 }
1028
1029 function s e tA re e na W al le t ( address _wa lletAd dress ) external onlyOwner {
1030 areenaWallet = _wal letAdd ress ;
1031 }
1032
1033 function setBusdWallet ( address _walle tAddre ss ) external onlyOwner {
1034 busdWallet = _wal letAdd ress ;
1035 }
1036
1037 function w i t h d r a w C o n t r a c t B a l a n c e ( uint256 _amount ) external onlyOwner {
1038
1039 require ( genToken . balanceOf ( address ( this ) ) >= _amount ,
1040 " Contract doesent have sufficient amount of Gen tokens to withdraw . " ) ;
1041
1042 genToken . transfer ( treasuryWallet , _amount ) ;
1043 }

Listing 3.6: Example Privileged Operations in StakingContract

93 function s e tP re S al e Pr ic e ( uint256 _newPrice ) external onlyOwner {


94 preSalePrice = _newPrice ;

17/20 PeckShield Audit Report #: 2022-369


Public

95 }

Listing 3.7: PreSale::setPreSalePrice()

192 function mint ( address account , uint256 amount ) public onlyOwner {


193
194 require ( account != address (0) , " ERC20 : mint to the zero address " ) ;
195
196 _totalSupply += amount ;
197 _balances [ account ] += amount ;
198 }
199
200 function burn ( address account , uint256 amount ) public onlyOwner {
201 require ( account != address (0) , " ERC20 : burn from the zero address " ) ;
202
203 uint256 ac countB alance = _balances [ account ];
204 _balances [ account ] = acco untBal ance - amount ;
205 _totalSupply -= amount ;
206 }

Listing 3.8: GenTokenCon::mint()/burn()

We emphasize that the privilege assignment may be necessary and consistent with the protocol
design. However, it is worrisome if the owner is not governed by a DAO-like structure. Note that a
compromised account would allow the attacker to modify a number of sensitive system parameters,
which directly undermines the assumption of the protocol design.

Recommendation Promptly transfer the privileged account to the intended DAO-like governance
contract. All changed to privileged operations may need to be mediated with necessary timelocks.
Eventually, activate the normal on-chain community-based governance life-cycle and ensure the in-
tended trustless nature and high-quality distributed governance.

Status The issue has been confirmed by the team. The team intends to introduce multi-sig
mechanism to mitigate this issue.

18/20 PeckShield Audit Report #: 2022-369


Public

4 | Conclusion

In this audit, we have analyzed the design and implementation of Battle Stakes, which is an innovative
DeFi protocol that is designed to be a hybrid of staking and play-to-earn project. It enables users
to earn BUSD, GEN (a native coin to the protocol), and ARENA (a token designed to increase in value
as the Battle Stakes network grows). It provides a new way to do yield farming by introducing
wallet against wallet staking. The current code base is well structured and neatly organized. Those
identified issues are promptly confirmed and addressed.
Meanwhile, we need to emphasize that smart contracts as a whole are still in an early, but exciting
stage of development. To improve this report, we greatly appreciate any constructive feedbacks or
suggestions, on our methodology, audit findings, or potential gaps in scope/coverage.

19/20 PeckShield Audit Report #: 2022-369


Public

References

[1] MITRE. CWE-287: Improper Authentication. https://cwe.mitre.org/data/definitions/287.html.

[2] MITRE. CWE-841: Improper Enforcement of Behavioral Workflow. https://cwe.mitre.org/data/

definitions/841.html.

[3] MITRE. CWE CATEGORY: 7PK - Security Features. https://cwe.mitre.org/data/definitions/

254.html.

[4] MITRE. CWE CATEGORY: Business Logic Errors. https://cwe.mitre.org/data/definitions/840.

html.

[5] MITRE. CWE VIEW: Development Concepts. https://cwe.mitre.org/data/definitions/699.html.

[6] OWASP. Risk Rating Methodology. https://www.owasp.org/index.php/OWASP_Risk_Rating_

Methodology.

[7] PeckShield. PeckShield Inc. https://www.peckshield.com.

20/20 PeckShield Audit Report #: 2022-369

You might also like