rubymem / bundler-leak

Known-leaky gems verification for bundler: `bundle leak` to check your app and find leaky gems in your Gemfile :gem::droplet:
https://www.rubymem.com
GNU General Public License v3.0
289 stars 11 forks source link

False Positives #10

Closed etagwerker closed 5 years ago

etagwerker commented 5 years ago

Hey @bronzdoc,

It seems that we have a problem with false positives. Some versions of some gems are known to be leaky, but it seems that bundler-leak is reporting them as leaky even if the project is using a patched version.

Case 1 (reported in the railsperf Slack)

"I just ran this against a project and I think I got two false-positives:"

zsh 2715  (git)-[paj/Introducing-bundler-leak]-% bundle leak
Name: redis
Version: 4.1.2
URL: https://github.com/redis/redis-rb/issues/612
Title: Memory Leak using Celluloid::Future
Solution: remove or disable this gem until a patch is available!

Name: sidekiq
Version: 5.2.7
URL: https://github.com/mperham/sidekiq/pull/2598
Title: Memory Leak in Sidekiq::Manager#real_thread
Solution: remove or disable this gem until a patch is available!

Vulnerabilities found!

"https://github.com/mperham/sidekiq/commit/d62ee8f9993512f26a405e7ac4431815b4230beb & https://github.com/redis/redis-rb/commit/d75708f6e05d9bba7dfec65880f9a55252f3fd2a Seem to have addressed both of these leaks in earlier versions than we’re running."

Case 2 (reported in the railsperf Slack)

"The result on my project"

ruby-mem-advisory-db: 9 advisories
Name: oj
Version: 3.6.4
URL: https://github.com/ohler55/oj/issues/229
Title: Memory Leak using Oj::Doc.open
Solution: remove or disable this gem until a patch is available!

Name: redis
Version: 4.0.2
URL: https://github.com/redis/redis-rb/issues/612
Title: Memory Leak using Celluloid::Future
Solution: remove or disable this gem until a patch is available!

Name: sidekiq
Version: 5.2.2
URL: https://github.com/mperham/sidekiq/pull/2598
Title: Memory Leak in Sidekiq::Manager#real_thread
Solution: remove or disable this gem until a patch is available!
bronzdoc commented 5 years ago

🤔 Will give it a look, thanks!

etagwerker commented 5 years ago

@bronzdoc Cool! Another case here:

"Hey Ernesto - just tried out bundler-leak. Cool stuff. I thing I may have one false positive with it."

Name: redcarpet
Version: 3.5.0
URL: https://github.com/vmg/redcarpet/pull/516
Title: Memory Leak in Redcarpet::Render::Base
Solution: remove or disable this gem until a patch is available!
tonydehnke commented 5 years ago

I got some that I think a false positives too:

Reading all 3 of the reported links, they are all several versions old and marked as merged or closed.

redis sidekiq redcarpet


Updating ruby-mem-advisory-db ...
From https://github.com/rubymem/ruby-mem-advisory-db
 branch            master     -> FETCH_HEAD
Already up to date.
Updated ruby-mem-advisory-db
ruby-mem-advisory-db: 9 advisories

Name: redcarpet
Version: 3.5.0
URL: https://github.com/vmg/redcarpet/pull/516
Title: Memory Leak in Redcarpet::Render::Base
Solution: remove or disable this gem until a patch is available!

Name: redis
Version: 4.1.2
URL: https://github.com/redis/redis-rb/issues/612
Title: Memory Leak using Celluloid::Future
Solution: remove or disable this gem until a patch is available!

Name: sidekiq
Version: 5.2.7
URL: https://github.com/mperham/sidekiq/pull/2598
Title: Memory Leak in Sidekiq::Manager#real_thread
Solution: remove or disable this gem until a patch is available!

Vulnerabilities found!`
jpanderson-outreach commented 5 years ago

Just another what looks like a false positive around oj? In case more data is helpful.

Name: oj
Version: 3.7.0
URL: https://github.com/ohler55/oj/issues/229
Title: Memory Leak using Oj::Doc.open
Solution: remove or disable this gem until a patch is available!
bronzdoc commented 5 years ago

Thanks so much @etagwerker @tonydehnke @jpanderson-outreach !

The issue lies in https://github.com/rubymem/ruby-mem-advisory-db Those gems don't have a patched version specified, so bundler-leak thinks they are leaky gems 😬

I'm working on a fix.

etagwerker commented 5 years ago

@tonydehnke @jpanderson-outreach I believe it is now fixed in master -- I just merged #11.

If it is not too much, could you test using master?

You can do that by adding this to your Gemfile:

group :development do 
  gem "bundler-leak", git: "https://github.com/rubymem/bundler-leak.git"

Then you can just run bundle exec bundle leak

jpanderson-outreach commented 5 years ago

So I ran

$ bundle exec bundle leak update
Updating ruby-mem-advisory-db ...
HEAD is now at d21d675 Init
Updated ruby-mem-advisory-db
ruby-mem-advisory-db: 9 advisories
$ bundle exec bundle leak
Name: oj
Version: 3.7.0
URL: https://github.com/ohler55/oj/issues/229
Title: Memory Leak using Oj::Doc.open
Solution: remove or disable this gem until a patch is available!

Name: redcarpet
Version: 3.4.0
URL: https://github.com/vmg/redcarpet/pull/516
Title: Memory Leak in Redcarpet::Render::Base
Solution: remove or disable this gem until a patch is available!

Name: redis
Version: 3.3.5
URL: https://github.com/redis/redis-rb/issues/612
Title: Memory Leak using Celluloid::Future
Solution: remove or disable this gem until a patch is available!

Vulnerabilities found!

and got the false positives still, even with pulling from git, including this as my Gemfile.lock:

GIT
  remote: https://github.com/rubymem/bundler-leak.git
  revision: 0551ce392ea5f95a345fa8d87d14987d8031119c
  specs:
    bundler-leak (0.0.0)
      bundler (>= 1.2.0, < 3)
      thor (~> 0.18)
bronzdoc commented 5 years ago

@jpanderson-outreach could you try now? i just merged https://github.com/rubymem/ruby-mem-advisory-db/pull/4 so it should work now.

tonydehnke commented 5 years ago

When I try to run it now I get:

Updating ruby-mem-advisory-db ...
From https://github.com/rubymem/ruby-mem-advisory-db
 * branch            master     -> FETCH_HEAD
fatal: refusing to merge unrelated histories
Failed updating ruby-mem-advisory-db!

I've run bundle update and tried to reinstall the gem too.

bronzdoc commented 5 years ago

@tonydehnke i just released bundler-leak 0.1.0, please try with that version and let us know if you still having issues, thanks!

tonydehnke commented 5 years ago

Re-ran in command: gem install bundler-leak Then ran bundle leak check --update

All seems to work now :) Thank you!

bundle leak check --update
Updating ruby-mem-advisory-db ...
HEAD is now at 231688a Merge pull request #4 from rubymem/add-leaky-gems-missing-fields
Updated ruby-mem-advisory-db
ruby-mem-advisory-db: 9 advisories
No vulnerabilities found
bronzdoc commented 5 years ago

Thank you @tonydehnke! let us know if you find anything else 🙇

I'm going to close this now.

23tux commented 5 years ago

When you just install it via gem install bundler-leak and directly run bundle leak afterwards, it still shows the false positives. So it doesn't work "out of the box", you still need a bundle leak check --update on your first run.

etagwerker commented 5 years ago

@23tux Thanks for your comment! I saw a similar comment in our blog, so I created https://github.com/rubymem/bundler-leak/issues/14 to keep track of that issue.