|
@@ -20,7 +20,7 @@ The git commit hash we evaluated is:
|
|
|
|
|
|
# Disclaimer
|
|
# Disclaimer
|
|
|
|
|
|
-The audit makes no statements or warrantees about utility of the code, safety of the code, suitability of the business model, regulatory regime for the business model, or any other statements about fitness of the contracts to purpose, or their bug free status. The audit documentation is for discussion purposes only.
|
|
|
|
|
|
+The audit makes no statements or warranties about utility of the code, safety of the code, suitability of the business model, regulatory regime for the business model, or any other statements about fitness of the contracts to purpose, or their bug free status. The audit documentation is for discussion purposes only.
|
|
|
|
|
|
# Executive Summary
|
|
# Executive Summary
|
|
|
|
|
|
@@ -90,7 +90,7 @@ We are still working through the confirmation protocol in `Shareable.sol`, but w
|
|
This bug has a number of causes that need to be addressed:
|
|
This bug has a number of causes that need to be addressed:
|
|
|
|
|
|
1. `resetSpentToday` and `confirm` together do not limit the days on which the function can be called or (it appears) the number of times it can be called.
|
|
1. `resetSpentToday` and `confirm` together do not limit the days on which the function can be called or (it appears) the number of times it can be called.
|
|
-1. Once a call has been confirmed and `execute`d it appears that it can be re-executed. This is not good.
|
|
|
|
|
|
+1. Once a call has been confirmed and executed it appears that it can be re-executed. This is not good.
|
|
3. `confirmandCheck` doesn't seem to have logic about whether or not the function in question has been called.
|
|
3. `confirmandCheck` doesn't seem to have logic about whether or not the function in question has been called.
|
|
4. Even if it did, `revoke` would need updates and logic to deal with revocation requests after a function call had been completed.
|
|
4. Even if it did, `revoke` would need updates and logic to deal with revocation requests after a function call had been completed.
|
|
|
|
|
|
@@ -109,7 +109,7 @@ It would be nice to see how many payments are pending. This would imply a bit of
|
|
|
|
|
|
## Shareable Contract
|
|
## Shareable Contract
|
|
|
|
|
|
-We do not believe the `Shareable.sol` contract is ready for primetime. It is missing functions, and as written may be vulnerable to a reordering attack -- an attack in which a miner or other party "racing" with a smart contract participant inserts their own information into a list or mapping.
|
|
|
|
|
|
+We do not believe the `Shareable.sol` contract is ready for prime time. It is missing functions, and as written may be vulnerable to a reordering attack -- an attack in which a miner or other party "racing" with a smart contract participant inserts their own information into a list or mapping.
|
|
|
|
|
|
The confirmation and revocation code needs to be looked over with a very careful eye imagining extraordinarily bad behavior by shared owners before this contract can be called safe.
|
|
The confirmation and revocation code needs to be looked over with a very careful eye imagining extraordinarily bad behavior by shared owners before this contract can be called safe.
|
|
|
|
|
|
@@ -159,7 +159,7 @@ Allows owner to set a public string of contract information. No issues.
|
|
|
|
|
|
This needs some work. Doesn't check if `_required <= len(_owners)` for instance, that would be a bummer. What if _required were like `MAX - 1`?
|
|
This needs some work. Doesn't check if `_required <= len(_owners)` for instance, that would be a bummer. What if _required were like `MAX - 1`?
|
|
|
|
|
|
-I have a general concern about the difference between `owners`, `_owners`, and `owner` in `Ownable.sol`. I recommend "Owners" be renamed. In general we do not recomment single character differences in variable names, although a preceding underscore is not uncommon in Solidity code.
|
|
|
|
|
|
+I have a general concern about the difference between `owners`, `_owners`, and `owner` in `Ownable.sol`. I recommend "Owners" be renamed. In general we do not recommend single character differences in variable names, although a preceding underscore is not uncommon in Solidity code.
|
|
|
|
|
|
Line 34: "this contract only has six types of events"...actually only two.
|
|
Line 34: "this contract only has six types of events"...actually only two.
|
|
|
|
|
|
@@ -224,7 +224,7 @@ Transfer() and transferFrom() use SafeMath functions, which will cause them to t
|
|
|
|
|
|
### SimpleToken
|
|
### SimpleToken
|
|
|
|
|
|
-Sample instantiation of StandardToken. Note that in this sample, decimals is 18 and supply only 10,000, so the supply is a small fraction of a single nominal token.
|
|
|
|
|
|
+Sample instantiation of StandardToken. Note that in this sample, decimals is 18 and supply is only 10,000, so the supply is a small fraction of a single nominal token.
|
|
|
|
|
|
### CrowdsaleToken
|
|
### CrowdsaleToken
|
|
|
|
|