PeckShield Audit Report Suterusu v1.0
PeckShield Audit Report Suterusu v1.0
SUTERUSU
PeckShield
March 1, 2021
Document Properties
Client Suterusu
Title Smart Contract Audit Report
Target Suterusu
Version 1.0
Author Xuxian Jiang
Auditors Huaguo Shi, Xuxian Jiang
Reviewed by Shuxiao Wang
Approved by Xuxian Jiang
Classification Public
Version Info
Contact
For more information about this document and its contents, please contact PeckShield Inc.
Contents
1 Introduction 4
1.1 About Suterusu . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 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 Incompatibility with Deflationary/Rebasing Tokens . . . . . . . . . . . . . . . . . . . 11
3.2 Possible Front-Running For Nonce Invalidation . . . . . . . . . . . . . . . . . . . . . 12
3.3 Improved Sanity Checks For System Parameters . . . . . . . . . . . . . . . . . . . . 14
3.4 Improved Ether Transfers . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 16
3.5 Suggested Adherence Of The Checks-Effects-Interactions Pattern . . . . . . . . . . . 17
3.6 Support Of Chain-Specific User Registration . . . . . . . . . . . . . . . . . . . . . . 18
3.7 Trust Issue of Admin Keys . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 19
4 Conclusion 21
References 22
1 | Introduction
Given the opportunity to review the design document and related smart contract source code of the
Suterusu 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.
Item Description
Issuer Suterusu
Website https://suterusu.io/
Type Ethereum Smart Contract
Platform Solidity
Audit Method Whitebox
Latest Audit Report March 1, 2021
In the following, we show the Git repository of reviewed files and the commit hash value used
in this audit. Note that the foundation of the entire protocol relies on the proposed efficient range
proof scheme with transparent setup. And the proof and correctness of this scheme is not part of
this audit.
• https://github.com/suterusu-team/suterusu-protocol.git (f883c1b)
Likelihood
1.3 Methodology
To standardize the evaluation, we define the following terminology based on OWASP Risk Rating
Methodology [12]:
• Impact measures the technical loss and business damage of a successful attack;
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
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) [11], 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.
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.
2 | Findings
2.1 Summary
Here is a summary of our findings after analyzing the Suterusu 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 logics, examine system operations, and place DeFi-related aspects under scrutiny to uncover
possible pitfalls and/or bugs.
Severity # of Findings
Critical 0
High 0
Medium 1
Low 4
Informational 2
Total 7
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.
Besides recommending specific countermeasures to mitigate these issues, we also emphasize that
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 need
to kick in at the very moment when the contracts are being deployed in mainnet. Please refer to
Section 3 for details.
3 | Detailed Results
Description
In the Suterusu protocol, the SuterERC20 contract is designed to be the main entry for ERC20-
related interaction with participating users. In particular, one entry routine, i.e., fund(), accepts
user deposits of supported assets (e.g., DAI). Naturally, the contract implements a number of low-
level helper routines to transfer assets in or out of the SuterERC20 contract. These asset-transferring
routines work as expected with standard ERC20 tokens: namely the internal asset balances are always
consistent with actual token balances maintained in individual ERC20 token contract.
22 f u n c t i o n f u n d ( U t i l s . G 1 P o i n t memory y , u i n t 2 5 6 unitAmount , b y t e s memory e n c G u e s s )
public {
23 f u n d B a s e ( y , unitAmount , e n c G u e s s ) ;
24
25 u i n t 2 5 6 n a t i v e A m o u n t = t o N a t i v e A m o u n t ( unitAmount ) ;
26
27 // In order for the following to succeed , ‘ msg . sender ‘ have to first approve ‘
this ‘ to spend the nativeAmount .
28 r e q u i r e ( t o k e n . t r a n s f e r F r o m ( msg . sender , a d d r e s s ( t h i s ) , n a t i v e A m o u n t ) , " Native ’
transferFrom ’ failed . " ) ;
29 }
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 a certain fee for every transfer
() or transferFrom(). (Another type is rebasing tokens such as YAM.) As a result, this may not meet
Status This issue has been confirmed. However, considering the fact that this specific issue
does not affect the normal operation, the team decides to address it when the need of supporting
deflationary/rebasing tokens arises.
Description
The SuterBase contract provides a transfer() function that allows users to authorize transfer by
using the Suterusu protocol. The function has a nonce-based mechanism to detect and block replay
attacks. To elaborate, we show below the implementation of the transfer() function.
Particularly, this function implements a rather straightforward logic in firstly validating the given
arguments, next properly transferring the internal balances of involved accounts, then checking the
freshness of the given nonce, and finally collecting the transfer fee (in addition to returned remaining
assets back to the user).
279 f u n c t i o n t r a n s f e r ( U t i l s . G 1 P o i n t [ ] memory C , U t i l s . G 1 P o i n t memory D,
280 U t i l s . G 1 P o i n t [ ] memory y , U t i l s . G 1 P o i n t memory u ,
281 b y t e s memory p r o o f ) p u b l i c p a y a b l e {
283 uint256 s t a r t G a s = g a s l e f t () ;
285 // TODO : check that sender and receiver should NOT be equal .
286 uint256 s i z e = y . length ;
301 s c r a t c h = a c c [ yHash ] ;
302 CLn [ i ] = s c r a t c h [ 0 ] . pAdd (C [ i ] ) ;
303 CRn [ i ] = s c r a t c h [ 1 ] . pAdd (D) ;
304 }
324 emit T r a n s f e r O c c u r r e d ( y ) ;
325 }
During our analysis, we observe that the calculated nonce (line 36) is checked against the list of
received nonce set, i.e., whether the new one is in the set of nonceSet (lines 307 − 309). If yes, it
reverts the transaction by reporting back stale nonce.
Here comes the problem: when an user intends to invoke transfer() to perform the asset transfer
by signing the transaction offline, but before the transaction is mined, it is possible for a malicious
actor to observe it (by closely monitoring the transaction pool) and then possibly front-runs it by
crafting a new transaction (with the same nonce) and offering a higher gas fee for block inclusion.
The new transaction may perform a fresh transfer()/burn(0) call. If the front-running is successful,
the crafted transaction essentially makes the given nonce included in the internal set, effectively
invalidating the user transaction that is being front-run.
Recommendation It may be desirable to not expose the plain-text for the nonce calculation.
Fortunately, this issue is mitigated as all nonces will be invalidated after the roll-over successfully
advances the epoch.
Status This issue has been confirmed. Considering the difficulty and possible lean gains in
exploiting the front-running, we agree with the team in keeping it as is.
Description
DeFi protocols typically have a number of system-wide parameters that can be dynamically config-
ured on demand. The Suterusu protocol is no exception. Specifically, if we examine the SuterBase
contract, it has defined a number of protocol-wide risk parameters, e.g., BURN_FEE_MULTIPLIER,
TRANSFER_FEE_MULTIPLIER, setEpochBase, and setEpochLength. The first two fee parameters affect the
fees that have been charged on burn and transfer respectively and the last two defines how epochs are
being computed. In the following, we show the corresponding routines that allow for their changes.
102 function setBurnFeeStrategy ( uint256 m u l t i p l i e r , uint256 d i v i d e n d ) p u b l i c {
103 r e q u i r e ( msg . s e n d e r == s u t e r A g e n c y , " Permission denied : Only admin can change
burn fee strategy . " ) ;
104 BURN_FEE_MULTIPLIER = m u l t i p l i e r ;
105 BURN_FEE_DIVIDEND = d i v i d e n d ;
106 }
107
108 // function c h a n g e T r a n s f e r F e e S t r a t e g y ( uint256 multiplier , uint256 dividend , uint256
nonce , uint256 c , uint256 s ) public {
109 // require (! u s e d F e e S t r a t e g y N o n c e s [ nonce ] , " Fee strategy nonce has been used !") ;
110 // u s e d F e e S t r a t e g y N o n c e s [ nonce ] = true ;
111
112 // Utils . G1Point memory K = Utils . g () . pMul ( s ) . pAdd ( s u t e r A g e n c y P u b l i c K e y . pMul ( c .
gNeg () ) ) ;
Our result shows the update logic on these fee parameters can be improved by applying more
rigorous sanity checks. Based on the current implementation, certain corner cases may lead to an
undesirable consequence. For example, an unlikely mis-configuration of a large fee parameter (say
more than 100%) will revert the transfer() operation.
Description
As described in Section 3.1, assets are transferred in or out with a number of helper routines such as
transfer() and transferFrom(). While dealing with ERC20 tokens, we have examined related helper
routines in their handling of non-standard ERC20 implementations. As for the case of transferring
ETH, the Solidity function transfer() is used (lines 29∕32 in the code snippet below). However, as
described in [1], when the recipient happens to be a contract that implements a callback function
containing EVM instructions such as SLOAD, the 2300 gas supplied with transfer() might not be
sufficient, leading to an out-of-gas error.
22 f u n c t i o n b u r n ( U t i l s . G 1 P o i n t memory y , u i n t 2 5 6 unitAmount , U t i l s . G 1 P o i n t memory u ,
b y t e s memory p r o o f , b y t e s memory e n c G u e s s ) p u b l i c {
23 u i n t 2 5 6 n a t i v e A m o u n t = t o N a t i v e A m o u n t ( unitAmount ) ;
24 u i n t 2 5 6 f e e = n a t i v e A m o u n t ∗ BURN_FEE_MULTIPLIER / BURN_FEE_DIVIDEND ;
26 b u r n B a s e ( y , unitAmount , u , p r o o f , e n c G u e s s ) ;
28 i f ( f e e > 0) {
29 suterAgency . transfer ( fee ) ;
30 totalBurnFee = totalBurnFee + fee ;
31 }
32 msg . s e n d e r . t r a n s f e r ( n a t i v e A m o u n t −f e e ) ;
33 }
As suggested in [1], we may consider avoiding the direct use of Solidity’s transfer() as well. Note
that we need to exercise extra caution during the use of call() as it may lead to side effects such as
re-entrancy and gas token vulnerabilities. In other words, we need to specify the maximum allowed
gas amount when making the (untrusted) external call().
Description
A common coding best practice in Solidity is the adherence of checks-effects-interactions principle.
This principle is effective in mitigating a serious attack vector known as re-entrancy. Via this
particular attack vector, a malicious contract can be reentering a vulnerable contract in a nested
manner. Specifically, it first calls a function in the vulnerable contract, but before the first instance
of the function call is finished, second call can be arranged to re-enter the vulnerable contract by
invoking functions that should only be executed once. This attack was part of several most prominent
hacks in Ethereum history, including the DAO [15] exploit, and the recent Uniswap/Lendf.Me hack [14].
We notice there are several occasions the checks-effects-interactions principle is violated. Using
the SuterERC20 as an example, the burn() function (see the code snippet below) is provided to
externally call a token contract to transfer assets. However, the invocation of an external contract
requires extra care in avoiding the above re-entrancy.
Apparently, the interaction with the external contract (line 38) starts before effecting the update
on internal states (line 39), hence violating the principle. In this particular case, if the external
contract has certain hidden logic that may be capable of launching re-entrancy via the very same
burn() function.
31 f u n c t i o n b u r n ( U t i l s . G 1 P o i n t memory y , u i n t 2 5 6 unitAmount , U t i l s . G 1 P o i n t memory u ,
b y t e s memory p r o o f , b y t e s memory e n c G u e s s ) p u b l i c {
32 u i n t 2 5 6 n a t i v e A m o u n t = t o N a t i v e A m o u n t ( unitAmount ) ;
33 u i n t 2 5 6 f e e = n a t i v e A m o u n t ∗ BURN_FEE_MULTIPLIER / BURN_FEE_DIVIDEND ;
34
35 b u r n B a s e ( y , unitAmount , u , p r o o f , e n c G u e s s ) ;
36
37 i f ( f e e > 0) {
38 r e q u i r e ( t o k e n . t r a n s f e r ( s u t e r A g e n c y , f e e ) , " Fail to charge fee . " ) ;
39 totalBurnFee = totalBurnFee + fee ;
40 }
41 r e q u i r e ( t o k e n . t r a n s f e r ( msg . sender , n a t i v e A m o u n t − f e e ) , " Fail to transfer tokens
." ) ;
42 }
In the meantime, we should mention that the supported tokens in the protocol do implement
rather standard ERC20 interfaces and their related token contracts are not vulnerable or exploitable
for re-entrancy.
Status The issue has been confirmed. The team plans to address it in the next upgrade.
Description
In the Suterusu protocol, there is a dedicated function register() to on-board a participating account.
It comes to our attention that the challenge calculation takes the following form: challenge = uint256
(keccak256(abi.encode(address(this), y, K))).gMod() (line 149). It fails to take into account the
important chainID information, which makes the differentiation from different Ethereum-alike chains
impossible.
146 f u n c t i o n r e g i s t e r ( U t i l s . G 1 P o i n t memory y , u i n t 2 5 6 c , u i n t 2 5 6 s ) p u b l i c {
147 // allows y to participate . c , s should be a Schnorr signature on " this "
148 U t i l s . G 1 P o i n t memory K = U t i l s . g ( ) . pMul ( s ) . pAdd ( y . pMul ( c . gNeg ( ) ) ) ;
149 u i n t 2 5 6 c h a l l e n g e = u i n t 2 5 6 ( keccak256 ( a b i . e n c o d e ( a d d r e s s ( t h i s ) , y , K) ) ) . gMod ( ) ;
150 r e q u i r e ( c h a l l e n g e == c , " Invalid registration signature ! " ) ;
151 b y t e s 3 2 yHash = keccak256 ( a b i . e n c o d e ( y ) ) ;
152 r e q u i r e ( ! r e g i s t e r e d ( yHash ) , " Account already registered ! " ) ;
153 // pending [ yHash ] = [y , Utils . g () ]; // " not supported " yet , have to do the below
154
155 /*
156 The following initial value of pending [ yHash ] is equivalent to an ElGamal
encryption of m = 0 , with nonce r = 1:
157 ( mG + ry , rG ) --> (y , G )
158 If we don ’t set pending in this way , then we can ’t differentiate two cases :
159 1. The account is not registered ( both acc and pending are 0 , because ‘
mapping ‘ has initial value for all keys )
160 2. The account has a total balance of 0 ( both acc and pending are 0)
161
162 With such a setting , we can guarantee that , once an account is registered ,
its ‘acc ‘ and ‘ pending ‘ can never ( c r y t o g r a p h i c a l l y negligible ) BOTH
equal to Point zero .
163 NOTE : ‘ pending ‘ can be reset to Point zero after a roll over .
164 */
165 p e n d i n g [ yHash ] [ 0 ] = y ;
166 p e n d i n g [ yHash ] [ 1 ] = U t i l s . g ( ) ;
167
168 totalUsers = totalUsers + 1;
169 }
Description
In the Suterusu protocol, there is an administrative-level account suterAgency that plays a critical role
in governing and regulating the system-wide operations (e.g., fee collection, and parameter setting).
Our analysis shows that the privileged account needs to be scrutinized. In the following, we examine
the privileged account and their related privileged accesses in current contracts.
To elaborate, we show below the implementation of burn() function. We notice a burn fee may
be charged and transferred to the privileged suterAgency account.
22 f u n c t i o n b u r n ( U t i l s . G 1 P o i n t memory y , u i n t 2 5 6 unitAmount , U t i l s . G 1 P o i n t memory u ,
b y t e s memory p r o o f , b y t e s memory e n c G u e s s ) p u b l i c {
23 u i n t 2 5 6 n a t i v e A m o u n t = t o N a t i v e A m o u n t ( unitAmount ) ;
24 u i n t 2 5 6 f e e = n a t i v e A m o u n t ∗ BURN_FEE_MULTIPLIER / BURN_FEE_DIVIDEND ;
26 b u r n B a s e ( y , unitAmount , u , p r o o f , e n c G u e s s ) ;
28 i f ( f e e > 0) {
29 suterAgency . transfer ( fee ) ;
30 totalBurnFee = totalBurnFee + fee ;
31 }
32 msg . s e n d e r . t r a n s f e r ( n a t i v e A m o u n t −f e e ) ;
33 }
As mentioned in Section 3.4, the ETH fee is collected via the Solidity function transfer() is
used (line 29). And if the recipient happens to be a contract that implements a callback function
containing EVM instructions such as SLOAD, the 2300 gas supplied with transfer() might not be
sufficient, leading to an out-of-gas error. This error could further revert the burn() operation, hence
potentially locking up user funds.
More specifically, a compromised suterAgency account would allow the attacker to add a malicious
suterAgency to lock up the funds. Also, this privileged account has the authority to make various
changes to a number of risk parameters (Section 3.3).
Recommendation Promptly transfer the privileged account to the intended DAO-like governance
contract. All changed to privileged operations need to be mediated with necessary timelocks. Even-
tually, activate the normal on-chain community-based governance life-cycle and ensure the intended
trustless nature and high-quality distributed governance.
Status This issue has been confirmed. The team confirmed the plan to hold the admin key in
a multi-sig account. All changed to privileged operations will be mitigated with necessary timelocks.
4 | Conclusion
In this audit, we have analyzed the design and implementation of the Suterusu protocol. The audited
system presents a unique addition by providing a new efficient range proof scheme with transparent
setup. The current code base is clearly organized and those identified issues are promptly confirmed
and fixed.
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.
References
09/stop-using-soliditys-transfer-now/.
[2] MITRE. CWE-1126: Declaration of Variable with Unnecessarily Wide Scope. https://cwe.
mitre.org/data/definitions/1126.html.
mitre.org/data/definitions/663.html.
[5] MITRE. CWE-754: Improper Check for Unusual or Exceptional Conditions. https://cwe.mitre.
org/data/definitions/754.html.
data/definitions/841.html.
254.html.
1006.html.
840.html.
html.
Rating_Methodology.
[14] PeckShield. Uniswap/Lendf.Me Hacks: Root Cause and Loss Analysis. https://medium.com/
@peckshield/uniswap-lendf-me-hacks-root-cause-and-loss-analysis-50f3263dcc09.
understanding-dao-hack-journalists.