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

Divide by zero false positives (can't ignore) #1103

Closed
GUI opened this Issue Sep 26, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@GUI

GUI commented Sep 26, 2017

I'm seeing some false-positive issues for "Potential divide by zero" errors in Brakeman's reports. Unlike warnings, it doesn't appear like there's a way to ignore these false positive errors in the "brakeman.ignore" ignore file.

The issue seems to specifically affect calculations using values inside of hash variables, but if I use local variables, then Brakeman does not trigger errors.

This was happening in Brakeman 3.7 too, but I hadn't been aware of it until the exit code changes in Brakeman 4. The only way I've found to ignore these errors in Brakeman 4 is to set the --no-exit-on-error flag, but ideally I actually would like the new exit code behavior (for other potential errors besides these). So I guess my question is whether there's a way to fix Brakeman's detection of divide by zero issues, or if there's some way to explicitly setup ignores for these errors when there are false positives.

Here's a simplified example demonstrating the issue:

module Zero
  # Local variables - no errors
  def test_local
    numerator = 0
    denominator = 0
    10.times do
      denominator += 2
    end

    numerator / denominator
  end

  # Hash variables - Brakeman error
  def test_hash
    @foo = {
      :numerator => 0,
      :denominator => 0,
    }
    10.times do
      @foo[:denominator] += 2
    end

    @foo[:numerator] / @foo[:denominator]
  end

  # Hash variables, with explicit check for 0 values - Brakeman error
  def test_hash_guard
    @foo = {
      :numerator => 0,
      :denominator => 0,
    }
    10.times do
      @foo[:denominator] += 2
    end

    if @foo[:denominator] != 0
      @foo[:numerator] / @foo[:denominator]
    end
  end
end

That will generate the following error report from Brakeman (with the local variable example not generating an error, but the hash examples both generating errors):

== Errors ==

Error: Potential divide by zero: (0 / 0) (line 23)
Location: 

Error: Potential divide by zero: (0 / 0) (line 37)
Location: 
@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Sep 26, 2017

Owner

Hi Nick,

Yes, these aren't really "false positives", they are errors raised during Brakeman's analysis. I think in this case I can fix them, though. Let me take a look.

Owner

presidentbeef commented Sep 26, 2017

Hi Nick,

Yes, these aren't really "false positives", they are errors raised during Brakeman's analysis. I think in this case I can fix them, though. Let me take a look.

@presidentbeef presidentbeef added this to the 4.0.2 milestone Nov 17, 2017

presidentbeef added a commit that referenced this issue Nov 17, 2017

presidentbeef added a commit that referenced this issue Nov 18, 2017

Add optional divide by zero check
instead of reporting an error as requested in #1103

Repository owner locked and limited conversation to collaborators Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.