Removing a file access warning

[akimd]

This is just a duplicate of #698.

Is your feature request related to a problem? Please describe. I am using a function that is sufficient to protect me from dangerous games with file names, as in #698, or in http://gavinmiller.io/2016/creating-a-secure-sanitization-function/, etc.

I know that:

• brakeman cannot decide whether my sanitizing function is safe
• but that's something I I would like to teach it via a --safe-file-sanitizer option or something equivalent. I'm not taking more chances by declaring that a sanitizer is safe, than when teaching brakeman how to ignore some warnings, even "High" ones.
• each time this was submitted here, the suggestion was redirected to "use ignores". But ignores are impractical in my case, as I generate many files from templates, and I do not want to have to teach brakeman about each occurrence. The hash sum makes it particularly painful to generate. The fact that I cannot simply leave a comment in the files (as is the case with most linters out there, except brakeman) is extremely troublesome.
• I don't want to just leave those warnings there: when I deliver the product, when customers look at brakeman's results, I don't want to have to tell them for each remaining line "no, that's a false positive".

Describe the solution you'd like I would like to have a means to give brakeman a list of sanitizers.

Describe alternatives you've considered Better yet would be a means to disable diagnostics locally, the brakeman.ignore approach is not adequate for my case. I'd be happy to be able to generate brakeman.ignore items from these comments, if that simplifies the implementation.

Additional context My whole business is about generating correct files automatically. Brakeman requires human interaction at places where automation would save me from a tedious and error-prone process.

Cheers!

[presidentbeef]

Hi Akim,

Thank you for bringing up this issue.

I have resisted adding more options like --safe-.... because there is no end to the possible options that could be added. There's also a lot of work to have those options work consistently across all checks.

In cases like file access, though, there is no standard way of "safely" accessing the file system, so I agree there should be some way of specifying that you would like to ignore a certain "safe" method.

What I am thinking about adding is a generic option to specify safe methods per check. For example:

--ignore-methods FileAccess:my_path_sanitizer

Something like that. Syntax to-be-determined.

It's still going to be a bit of work to implement and maintain that option across checks, but at least it wouldn't require implementing many different options.

Would that address your use case?

Better yet would be a means to disable diagnostics locally, the brakeman.ignore approach is not adequate for my case.

I assume you mean via inline comments? Sorry, but I have no interest in doing so. This has been debated many times.

[akimd]

Hi Justin,

Thanks for answering. Sorting sanitizers by category looks promising.

I assume you mean via inline comments? Sorry, but I have no interest in doing so. This has been debated many times.

Yes, I know. But I would answer that there's a reason why it's coming up frequently.

Since the current approach via an ignore file is unsuitable for my case, and probably others', I'm now using something like the appended script. The idea is simple: use some fake function (I use brakeman_ignore which merely returns it argument) to flag calls you want brakeman to ignore. Then run brakeman and have it generate the ignores for all the warnings, and finally just keep the ones you are interested in.

Currently the approach is crude, but it's working. The following script expects to be run from the root of the Rails project, and expect config/brakeman.ignore.ref to contains the hand-tuned ignores.

#! /usr/bin/env ruby

require 'json'

cmd = 'brakeman'
ign = 'config/brakeman.ignore'

script = <<EOF
#{ign}.ref # Input file
2          # Hide previously ignored warnings
a          # Ignore this warning and all remaining warnings
no         # Remove fingerprints?
1          # Save changes
#{ign}     # Output file
EOF

# Reset to empty.
open(ign, 'w').close

res = IO.popen("#{cmd} -I", 'r+') do |io|
  script = script.gsub(/\s*#.*/, '')
  puts "Send: #{script}"
  io.write(script + "\n")
  io.close_write
  io.read
end
puts "brakeman said: #{res.gsub(/^/, '      ')}"

new_ignores = File.open(ign, 'r') { |file; config|
  config = JSON.load(file)
  config['ignored_warnings'].select { |warn|
    warn['code'] =~ /brakeman_ignore/
  }
}

ignores = File.open("#{ign}.ref", 'r') { |file; res|
  res = JSON.load(file)
  res['ignored_warnings'].append(*new_ignores)
  res
}

File.open(ign, 'w') do |file|
  file.write(JSON.pretty_generate(ignores))
end

puts "Last run"
system(cmd)

Cheers.