crytic / slither

Static Analyzer for Solidity and Vyper
https://blog.trailofbits.com/2018/10/19/slither-a-solidity-static-analysis-framework/
GNU Affero General Public License v3.0
5.15k stars 947 forks source link

[False-Positive]: Weak PRNG Usage #2267

Open MarkLee131 opened 6 months ago

MarkLee131 commented 6 months ago

Describe the false alarm that Slither raise and how you know it's inaccurate:

Slither reports Weak PRNG when it found there are some module usage %, regardless of it depends the block.timestamp.

Frequency

Very Frequently

Code example to reproduce the issue:

In function rpow:

function rpow(uint x, uint n, uint b) internal pure returns (uint z) {
  assembly {
    switch x case 0 {switch n case 0 {z := b} default {z := 0}}
    default {
      switch mod(n, 2) case 0 { z := b } default { z := x }
      let half := div(b, 2)  // for rounding.
      for { n := div(n, 2) } n { n := div(n,2) } {
        let xx := mul(x, x)
        if iszero(eq(div(xx, x), x)) { revert(0,0) }
        let xxRound := add(xx, half)
        if lt(xxRound, xx) { revert(0,0) }
        x := div(xxRound, b)
        if mod(n,2) {
          let zx := mul(z, x)
          if and(iszero(iszero(x)), iszero(eq(div(zx, x), z))) { revert(0,0) }
          let zxRound := add(zx, half)
          if lt(zxRound, zx) { revert(0,0) }
          z := div(zxRound, b)
        }
      }
    }
  }
}

It got:

INFO:Detectors:
Jug.rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "switch_expr_2366_53_0_rpow_asm_0 = n % 2 (jug.sol#66)" 
Jug.rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "n % 2 (jug.sol#74-80)"
function add(uint x, uint y) internal pure returns (uint z) {
    z = x + y;
    z = z % 2;
    require(z >= x);
}

It reported:

Jug.add(uint256,uint256) (jug.sol#86-90) uses a weak PRNG: "z = z % 2 (jug.sol#88)"

Version:

0.10.0

Relevant log output:

INFO:Detectors:
Jug.rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "switch_expr_2366_53_0_rpow_asm_0 = n % 2 (jug.sol#66)" 
Jug.rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "n % 2 (jug.sol#74-80)" 
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#weak-PRNG
sriramsowmithri9807 commented 6 months ago

it seems that Slither is raising a false alarm regarding the detection of a weak Pseudo-Random Number Generator (PRNG) in the code. Let's break down the key points:

  1. Nature of False Alarm:

    • Slither is reporting a weakness in the PRNG used in the rpow function of the Jug contract.
    • The reported weak PRNG is based on the use of the modulo operation (n % 2) in the switch statement and in the subsequent loop.
    • Slither is flagging the usage of % 2 as a weak PRNG, but the provided code doesn't seem to have an inherent weakness.
  2. Frequency:

    • The false alarm is reported very frequently, indicating that the detector is triggering on multiple instances of the supposed weak PRNG pattern.
  3. Code Example:

    • The provided code example includes the rpow function from the Jug contract and a simple case for the add function.
    • The reported weak PRNG is associated with the line switch mod(n, 2) case 0 { z := b } default { z := x } in the rpow function.
  4. Version and Relevant Log Output:

    • The Slither version used is 0.10.0.
    • The log output shows the specific lines where Slither is detecting the weak PRNG and references to the occurrences.
  5. Reference:

  6. Verification of False Positive:

    • To verify if it's a false positive, developers need to review the code and the reported weak PRNG instances.
    • Since the logic involves basic arithmetic operations (% 2), it's crucial to analyze whether it indeed poses a security risk in the context of the given code.

In conclusion, to confirm the false positive, you should carefully review the code and assess whether the reported weak PRNG instances are genuinely problematic or if Slither's detection mechanism might be overly sensitive in this case. The provided information doesn't seem to indicate a clear weakness in the PRNG logic.

MarkLee131 commented 6 months ago

R u a bot, bro?@sriramsowmithri9807

MarkLee131 commented 5 months ago

Hi @0xalpharush, really appreciate what you guys have done with Slither for Solidity smart contracts. It's been super helpful in my work. I'm eager to contribute and make it even better, and would love your take on this issue I'm looking at. Any chance you could give some feedback? Thanks a ton! :)