When “SafeTransfer” Becomes Unsafe: lessons from the QBridge security incident
On January 28, the QBridge was attacked and the asset values around 80M USDs were stolen. After the analysis, we found that the root cause in the code is the implementation of the safeTransfer
(and safeTransferFrom
) function.
The root cause
First, the project does not use the popular OpenZeppelin SafeERC20 library for the token transfer. Instead, they implemented a library called SafeToken.
Second, the implementation does not check the target is a valid contract (or whether it’s a zero address).
Third, the low-level call of the EVM does not return false when the target contract is zero. This contradicts the developer’s common sense.
Of course, the incident also has some other reasons, e.g., the zero address is put into the whitelist. However, if the code can properly handle this special case, then it’s not vulnerable.
BTW: There exist other projects using the similar code. They may be susceptible to the similar issue.
Experiment
To confirm the third reason that VM does not return false when the target contract is zero, we developed a test contract, as shown in the following.
The execution of TestSafeTransfer.test()
will not revert.
Lessons and how to mitigate the risk
We suggest that
- Use the popular library instead of inventing your own wheel, unless you have a very very good reason
- check the balance before and after the asset transfer to ensure that number of the transferred asset compiles the expectation. This can also avoid the issue of the deflation and inflation token.