Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Slither tool analysis report #727

Closed

Conversation

nabialek-arianelabs
Copy link

Adds Slither tool analysis, docker files, and POC for ecrecover Slither detector.
Is dependant upon: https://github.com/hashgraph/hedera-smart-contracts/pull/726/files

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: maciek.nabialek <[email protected]>
@quiet-node
Copy link
Member

Hi @nabialek-arianelabs, thanks for the nice work! With a quick scan through the CI, I noticed that there are two tests failed. Please follow the conventional commits guidelines to update the title of the PR to fix the PR Check / Title Check CI task. Another the DCO failed task, it looks like one of the commit in this PR didn't get signed off wit hthe Signed-off-by... message. One way to do it is to squash the commits and add the signature.

or [taint checking](https://en.wikipedia.org/wiki/Taint_checking).
General Slither architecture is shown below on the fig. 1.

![fig1_img.png](static/img.png)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the static/img.png has been removed? Should we keep the image to show here or should we remove this line?

},
...
```
Full json output is available [here](Slither/slither-read-storage-mainnet.json)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this Slither/slither-read-storage-mainnet.json file?

```
Execution of command:
`slither-read-storage 0x00000000000000000000000000000000002e7a5d --json storage_layout.json --rpc-url https://mainnet.hashio.io/api`
yields `ERROR:SlitherSolcParsing` with details available [here](slither_reports/slither-read-storage-hashscan-error.md).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link embedded in here won't work

#### Reports of the tests:
| Smart contract | Result |
|----------------------------------------------------------------------------------|--------------------------------------------------|
| [AtomicHTS.sol](test_contracts/hts-precompile/AtomicHTS.sol) | [Report](slither_reports/AtomicHTS.md) |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update these file paths

* Tests didn't generate any kind of high-impact vulnerability. The Table below depicts a detailed histogram
of the reported issues by the Slither (for hts-precompile contracts):

![slither_report_histogram.png](static/slither_report_histogram.png)\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image is not found

function setBit(uint256 self, uint8 index) internal pure returns (uint256) {
return self | (ONE << index);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing line at EOF

* Emits a {Transfer} event.
*/
function transferFrom(address from, address to, uint256 value) external returns (bool);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing line at EOF

* @dev Returns the decimals places of the token.
*/
function decimals() external view returns (uint8);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing line at EOF

}
}

//contract Attack {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is Attack commented out?

@@ -0,0 +1,42 @@
// SPDX-License-Identifier: MIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update license to Apache-2.0

@arianejasuwienas
Copy link
Contributor

Replaced with: #740

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants