yuriy77k
commented
6 years ago You could follow the audit queue there: https://github.com/EthereumCommonwealth/Auditing/blob/master/queue.md
Closed yuriy77k closed 6 years ago
yuriy77k
commented
6 years ago You could follow the audit queue there: https://github.com/EthereumCommonwealth/Auditing/blob/master/queue.md
RideSolo
commented
6 years ago Auditing time: ~ 7 days.
yuriy77k
commented
6 years ago @RideSolo assigned.
gorbunovperm
commented
6 years ago Estimated auditing time is 7 days.
yuriy77k
commented
6 years ago @gorbunovperm assigned
MrCrambo
commented
6 years ago Auditing time 7 days.
yuriy77k
commented
6 years ago @MrCrambo assigned
gorbunovperm
commented
6 years ago My report is finished.
yuriy77k
commented
6 years ago The contract contains high severity security issue. The report was sent to the developer.
yuriy77k
commented
6 years ago @MrCrambo Notes about you report:
mint() function. And possibility of owner to mint token does not mean that owner will mint. Possibility of increasing Tokens amount has low severity.yuriy77k
commented
5 years ago W12 Project security audit report performed by Callisto Security Audit Department
12 issues were reported including:
1 high severity issues.
3 medium severity issues.
5 low severity issues.
3 minor observation.
Without correction of the Refund logic any user will be able to be refunded a higher amount of ether than what he bought, making the last investors to call refund, lose their ether.
Each call to getRefundAmount will recalculate the average price of the tokens bought by the investor (following the ether left in the contract since tranche can withdraw some ether after every milestone reached) and multiply it by the amount of tokens to be refunded.
getRefundAmount function description do not match the implemented code (if this is not corrected the investor will be refunded more than what is expected), as shown below c is first multiplied by a than divided by b but the implemented code show the opposite:
/**
a = address(this).balance
b = totalFunded
c = buyers[buyer].totalFunded
d = buyers[buyer].totalBought
e = wtokensToRefund
( ( c * (a / b) ) / d ) * e = (refund amount)
*/ uint allowedFund = buyers[buyer].totalFunded.mul(totalFunded).div(address(this).balance);If the above issue is taken into consideration and changed, investors should be aware that the refund function is dependent on the remaining ether balance of the contract. In other terms the transactions order of investors calling refund or if tranche is called by the owner will impact the refund value of every investor.
The formula present a ratio of the total ethereum invested by a user divided by the total ethereum collected at the moment of the call of the function multiplied by the contract balance, since the contract balance will change after every call, the refund value of a user that will call refund after will be affected. Please note that the same issue can be applied to tranche function.
Another remark is with this logic the contract if the ICO fail and all users call refund, the contract will always have ether stuck in it.
The dependence can be easily avoided if the developers set a fixed amount to be redistributed to the investors and not use address(this).balance.
The white paper describe tranche function to allow the project to receive funds for each stage of the Roadmap, before the start of the sale of tokens:
"The project receives installments at the end of each stage of the Roadmap. For each stage of the Roadmap, before the start of the sale of tokens, the project specifies the% of the funds that it can receive, and the date from which it is possible to request these funds."
However tranche function is allowed to execute following the milestones not the stages.
addTokenToListing member of TokenExchanger contract pairs tokens to be exchanged in a 1:1 ratio only following TokenExchanger contract logic.
If an ERC20 or WToken address is paired with different addresses, the value of listingTokenToWToken[token](if the same token address is paired more than once) and listingWTokenToToken[wToken](if the same wToken address is paired more than once) will be overwritten since hasPair(token, wToken) will return false even if one of the addresses was used before, consequences:
getWTokenByToken will return the last paired value.getTokenByWToken will return the last paired value.exchange function might end up receiving a different token or the function might just revert.removePricer function member of PricerRole can be called by anyone and remove all addresses allowed with onlyPricer modifier, no access restriction is set.
Depending in the further use of PricerRole contract, the impact can vary widely.
removeSymbol function member of Symbols does not finish updating the last symbol that has been moved to the deleted index, which may returns an erroneous index later. Depending in the further use of Symbols contract, the impact can vary widely.
Another issue is that no access restriction is set in the contract, following the contract logic setup by the developers this might present a risk or not (code reusability can be affected).
function _removeSymbolByIndex(uint index) internal {
require(index < symbolsList.length);
if (index != symbolsList.length - 1) {
symbolsList[index] = symbolsList[symbolsList.length - 1];
// -- Add the following to update the index of the last symbol -- //
symbolIndex[symbolsList[index]] = index;
delete symbolsList[symbolsList.length - 1];
}
else delete symbolsList[index];
// --------------------- END RECOMMENDATION --------------------- //
symbolsList.length--;
}setVersion function member of VersionsLedger contract do not require _address input parameters to be different than zero address.
In WhitePaper at page 48 there written, that tokens amount are limited and can not be increased, but smart contract has mint() function and trusted people can mint, also owner can add trusted people.
There should be no possibility of minting extra tokens, because in whitepaper there written that tokens amount are limited and fixed.
vestingTransfer function with zero _value.The _value parameter is not checked for a zero value. In case if _value is zero the condition in line 182 will be true and a vesting time will be added to vestingTimes array each call of vestingTransfer function with zero of _value parameter. Thus, duplicate values will be accumulated in the vestingTimes array.
And if after zero value vestingTransfer will be transfer with positive value, then accountBalance function will not work correctly. For each duplicated vestingTime the balance will be subtracted.
It can happen by accident or maliciously.
Use check for zero value of _value parameter. Or check for duplicates in vestingTimes array.
TODO comment left in code.
Modifier onlyFrom allows everyone and can be deleted and not used.
Checking has no sense, because SafeMath library will do it automatically.
The audited contracts contain different severity level issues that need to be fixed before deployment including a high severity.
https://gist.github.com/yuriy77k/9bebeab10b57be632c6aa5592c5060af
https://gist.github.com/yuriy77k/0ac2e633c833e87abcb466807df21aec
https://gist.github.com/yuriy77k/430045c0823ecadacf952720cb62dda9
Audit request
A service has been developed for the release and sale of ICO tokens and for charity projects (ERC-20 on Ethereum). The project has the ability to sell all or only part of the issued tokens using the W12 platform. The main goal of the service is to increase transparency, increase trust between the parties and reduce risks arising from the financing of projects. After the end of each stage of the roadmap, the buyer of the tokens can take advantage of the opportunity to return their funds remaining in the project fund (if it considers that the project does not fulfill its obligations). The project funds are issued in parts after the end of the roadmap stage and the time allotted for the return of funds by the buyers of the tokens. Detailed description https://docs.google.com/document/d/1tEMEhHRREhJVKLYn-K2_83hLe2KQYvJUb2gA6r361JY/edit?usp=sharing Bounty W12 (code audit) https://w12.io/bounty-w12/section/5/
Source code
https://github.com/w12-platform/W12-Product-Blockchain-Protocol
Disclosure policy
andrey@w12.io
Platform
ETH
Complexity
High