0

While testing some contracts using Slither I have encountered 2 strange things.

  1. In below contract EtherStore.sol, which originally had reentrancy vulnerability I have came up with an idea that I will add ReentrancyGuard to it and test it with Slither again to see if vulnerability is gone, but it did not...

Any idea why is that? Slither is not implementing reentrancy guard or he is just pointing us that there is reentrancy in this function and we should cover it with ReentrancyGuard?


// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract EtherStore is ReentrancyGuard {
    mapping(address => uint256) public balances;

    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }

    function withdraw() external nonReentrant {
        uint256 balance = balances[msg.sender];
        require(balance > 0);
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success, "Failed to send Ether");
        balances[msg.sender] = 0;
    }

    // Helper function to check the balance of this contract
    function getBalance() external view returns (uint256) {
        return address(this).balance;
    }
}

  1. I have also came up with Slither catching errors in ERC721A, which in my opinion is a bit ridiculous as a lot of protocols using it... Any idea why Slither is yelling like below on ERC721A? Is it safe to use ERC721A and Slither is just wrong or there are vulnerabilities for real?

enter image description here

Thank you in advance for any feedback!

1 Answer 1

2

Actually, most of the vulnerabilities Slither gives out are ignorable. It just prints out every possible vulnerability, among which very few can be actually considered as a vulnerability.

It also points out a lot of vulnerabilities from OZ contracts, which are not vulnerable. You can ignore them unless the pointed-out lines actually seem vulnerable.

1
  • 1
    Thank you Sir! It was just a bit confusing to have errors in ERC721A..
    – Neftyr
    Commented Apr 28, 2023 at 10:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.