Skip to content
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

[CVE-2017-8418] Update ruby plugins for vulnerable rubocop dependency #77

Open
majormoses opened this issue Nov 20, 2017 · 15 comments
Open

Comments

@majormoses
Copy link
Member

majormoses commented Nov 20, 2017

UPDATE (2/6/18) - Please specifically version to ~> 0.51.0 for consistency across plugins.

Original post:

Update rubocop gems to 0.51+ to mitigate issue: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-8418 This is a very low severity warning (and agree after reading the impact) but it is a security one and could be a relatively easy to divide and conquer. If anyone would like to help out please comment here claiming which ones you will work on. I will start at the top and work my way down skipping any that are claimed. While we do not explicitly call out the use of ### Security in our changelog guidelines there is mention of such in the keep a changelog guidelines which we are based on.

Github is the best:
image

Here is a list of plugins:

  • sensu-plugins-ansible
  • sensu-plugins-apache
  • sensu-plugins-aws
  • sensu-plugins-beanstalk
  • sensu-plugins-bigpanda
  • sensu-plugins-bluepill
  • sensu-plugins-campfire
  • sensu-plugins-cassandra
  • sensu-plugins-ceph
  • sensu-plugins-cgroups
  • sensu-plugins-chatwork
  • sensu-plugins-chef
  • sensu-plugins-chrony
  • sensu-plugins-clockworksms
  • sensu-plugins-conntrack
  • sensu-plugins-consul
  • sensu-plugins-couchbase
  • sensu-plugins-couchdb
  • sensu-plugins-cpu-checks
  • sensu-plugins-cucumber
  • sensu-plugins-dashing
  • sensu-plugins-datadog
  • sensu-plugins-dcos
  • sensu-plugins-dhcp
  • sensu-plugins-disk-checks
  • sensu-plugins-dns
  • sensu-plugins-docker
  • sensu-plugins-dovecot
  • sensu-plugins-eep
  • sensu-plugins-elasticsearch
  • sensu-plugins-entropy-checks
  • sensu-plugins-environmental-checks
  • sensu-plugins-erlang
  • sensu-plugins-etcd
  • sensu-plugins-execute
  • sensu-plugins-eye
  • sensu-plugins-feature-requests
  • sensu-plugins-filesystem-checks
  • sensu-plugins-flowdock
  • sensu-plugins-fluentd
  • sensu-plugins-freeradius
  • sensu-plugins-ftp
  • sensu-plugins-gearman
  • sensu-plugins-geckoboard
  • sensu-plugins-gelf
  • sensu-plugins-github
  • sensu-plugins-gluster
  • sensu-plugins-golang
  • sensu-plugins-google-spreadsheet
  • sensu-plugins-gpg
  • sensu-plugins-graphite
  • sensu-plugins-graylog
  • sensu-plugins-growthforecast
  • sensu-plugins-haproxy
  • sensu-plugins-hardware
  • sensu-plugins-hbase
  • sensu-plugins-hipchat
  • sensu-plugins-http
  • sensu-plugins-hubot
  • sensu-plugins-icecast
  • sensu-plugins-iis
  • sensu-plugins-imap
  • sensu-plugins-imkayac
  • sensu-plugins-influxdb
  • sensu-plugins-io-checks
  • sensu-plugins-ipmi
  • sensu-plugins-ipvs
  • sensu-plugins-irc
  • sensu-plugins-java
  • sensu-plugins-jenkins
  • sensu-plugins-jolokia
  • sensu-plugins-kannel
  • sensu-plugins-kegbot
  • sensu-plugins-kubernetes
  • sensu-plugins-ldap
  • sensu-plugins-librato
  • sensu-plugins-load-checks
  • sensu-plugins-logs
  • sensu-plugins-logstash
  • sensu-plugins-lvm
  • sensu-plugins-lxc
  • sensu-plugins-mackerel
  • sensu-plugins-mailer
  • sensu-plugins-mailgun
  • sensu-plugins-memcached
  • sensu-plugins-memory-checks
  • sensu-plugins-mesos
  • sensu-plugins-messagemedia
  • sensu-plugins-microsoft-teams
  • sensu-plugins-mongodb
  • sensu-plugins-monit
  • sensu-plugins-mysql
  • sensu-plugins-nbzget
  • sensu-plugins-netscaler
  • sensu-plugins-network-checks
  • sensu-plugins-newrelic
  • sensu-plugins-nginx
  • sensu-plugins-nrpe
  • sensu-plugins-ntp
  • sensu-plugins-nvidia
  • sensu-plugins-officehours
  • sensu-plugins-openldap
  • sensu-plugins-openstack
  • sensu-plugins-opentsdb
  • sensu-plugins-openvpn
  • sensu-plugins-opsgenie
  • sensu-plugins-pacemaker
  • sensu-plugins-pagerduty
  • sensu-plugins-pdns
  • sensu-plugins-percona
  • sensu-plugins-php-fpm
  • sensu-plugins-pingdom
  • sensu-plugins-ponymailer
  • sensu-plugins-postfix
  • sensu-plugins-postgres
  • sensu-plugins-process-checks
  • sensu-plugins-puma
  • sensu-plugins-puppet
  • sensu-plugins-pushover
  • sensu-plugins-qmail
  • sensu-plugins-rabbitmq
  • sensu-plugins-raid-checks
  • sensu-plugins-redis
  • sensu-plugins-request-tracker
  • sensu-plugins-resque
  • sensu-plugins-rethinkdb
  • sensu-plugins-riak
  • sensu-plugins-riemann
  • sensu-plugins-rspec
  • sensu-plugins-selinux
  • sensu-plugins-sensu
  • sensu-plugins-sentry
  • sensu-plugins-serverspec
  • sensu-plugins-sftp
  • sensu-plugins-sidekiq
  • sensu-plugins-signifai
  • sensu-plugins-sip
  • sensu-plugins-skel
  • sensu-plugins-skyline
  • sensu-plugins-slack
  • sensu-plugins-sms
  • sensu-plugins-snmp
  • sensu-plugins-solr
  • sensu-plugins-splunk
  • sensu-plugins-springboot
  • sensu-plugins-ssl
  • sensu-plugins-stackstorm
  • sensu-plugins-statuspage
  • sensu-plugins-strongswan
  • sensu-plugins-supervisor
  • sensu-plugins-switchvox
  • sensu-plugins-syslog-ng
  • sensu-plugins-systemd
  • sensu-plugins-telapi
  • sensu-plugins-telegram
  • sensu-plugins-tempodb
  • sensu-plugins-tomcat
  • sensu-plugins-trafficserver
  • sensu-plugins-tripwire
  • sensu-plugins-twemproxy
  • sensu-plugins-twilio
  • sensu-plugins-twitter
  • sensu-plugins-ubiquiti
  • sensu-plugins-uchiwa
  • sensu-plugins-unicorn
  • sensu-plugins-ups
  • sensu-plugins-uptime-checks
  • sensu-plugins-varnish
  • sensu-plugins-victorops
  • sensu-plugins-vmstats
  • sensu-plugins-vsphere
  • sensu-plugins-windows
  • sensu-plugins-wordpress
  • sensu-plugins-xen
  • sensu-plugins-xmpp
  • sensu-plugins-zendesk
  • sensu-plugins-zfs
  • sensu-plugins-zookeeper
@majormoses
Copy link
Member Author

Prompted by this issue: #78 please leave your comments, thoughts, etc.

@thomasriley
Copy link

Picking up sensu-plugins-ansible

@majormoses
Copy link
Member Author

majormoses commented Nov 20, 2017

I started at the top so I am working on ansible, might be best for you to start at the bottom and work up.

@thomasriley
Copy link

@majormoses sounds like a plan 👍

@majormoses
Copy link
Member Author

Take a look at: sensu-plugins/sensu-plugins-ansible#6 which is an example and why I created the security issue linked above (avoiding link spam) one to discuss how to version it.

@thomasriley
Copy link

Seeing this erroneous rubocop failure with Rubocop 0.51:

bin/metrics-zookeeper-cluster.rb:52:60: C: Do not place comments on the same line as the def keyword.
  def follow_url(uri_str, agent = "sensu-plugins-zookeeper/#{SensuPluginsZookeeper::Version::VER_STRING}", max_attempts = 10, timeout = 10)
                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

There is a fix, but it has not been released yet: rubocop/rubocop#4886

Until that is released, shall we disable the Style/CommentedKeyword cop via .rubocop.yml ?

@majormoses
Copy link
Member Author

majormoses commented Nov 20, 2017

I'd prefer we disable it inline and link above it with a TODO: to remove it later. More specific disabling is generally better than blanket disabling it. It leaves it to the developer to treat rubocop like the pirates code. A knowledgeable developer will be able to look at the code and say that cop is wrong and not blindly find it there but realize there are other times where it is pushing you to write better code.

@thomasriley
Copy link

thomasriley commented Nov 20, 2017

Another is that Metrics/BlockLength is enabled by default in a newer version of Rubocop. A caveat I find with this one is, for example, a gemspec file is likely to exceed the default maximum 25 lines if you have a good few dependancies included. Tend to find with this cop, you have to be fairly pragmatic and disable as appropriate (e.g. gemspecs!). I'm doing this inline also, but we should discuss this further. Thoughts?

I've raised my first PR for this (sensu-plugins/sensu-plugins-zookeeper#15). Would appreciate a review before I crack on with anymore please, make sure I'm on the right track :)

I thought these would be a really quick change.. but of course significant bump in rubocop version means many more additional cops and therefore further amendments required.

@majormoses
Copy link
Member Author

majormoses commented Nov 20, 2017

I have been disabling it inline for the gemspec as there is no value there, there is good reason to keep blocks relatively short as they are basically being passed to a function. I don't want to make a blanket wide decision yet on this without analyzing the impact. In other cases if it's not a trivial refactor I'd disable it inline with a TODO comment.

I thought these would be a really quick change.. but of course significant bump in rubocop version means many more additional cops and therefore further amendments required.

Story of my life, the aws one was pretty gnarly.

@majormoses
Copy link
Member Author

majormoses commented Nov 21, 2017

I think we probably want to maybe tweak the default to say enable it but limit it to 50 (opposed to the 25) lines in a block. Honestly if you need 50 lines you really need helper functions. @sensu-plugins/commit-bit please weigh in.

@nicoleheejin
Copy link

nicoleheejin commented Jan 21, 2018

Planning on taking a look at this now! Believe I got it to pass, but please let me know if there's anything I should add. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants