Skip to content
This repository has been archived by the owner on Jan 1, 2020. It is now read-only.

A TTL value submitted as a string can cause the sensu-server to die. #1182

Closed
jbehrends opened this issue Mar 4, 2016 · 3 comments
Closed
Assignees

Comments

@jbehrends
Copy link

We recently had a sensu-server die for a few days unknowingly. The last log entry before dieing was this:

{"timestamp":"2016-03-04T09:45:18.687212-0800","level":"warn","message":"reconnecting to redis"}
/opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/sensu-0.20.1/lib/sensu/server/process.rb:672:in `>=': comparison of Fixnum with String failed (ArgumentError)
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/sensu-0.20.1/lib/sensu/server/process.rb:672:in `block (5 levels) in determine_stale_check_results'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/em-redis-unified-1.0.0/lib/em-redis/redis_protocol.rb:489:in `call'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/em-redis-unified-1.0.0/lib/em-redis/redis_protocol.rb:489:in `dispatch_response'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/em-redis-unified-1.0.0/lib/em-redis/redis_protocol.rb:436:in `process_cmd'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/em-redis-unified-1.0.0/lib/em-redis/redis_protocol.rb:408:in `receive_data'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/eventmachine-1.0.3/lib/eventmachine.rb:187:in `run_machine'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/eventmachine-1.0.3/lib/eventmachine.rb:187:in `run'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/sensu-0.20.1/lib/sensu/server/process.rb:23:in `run'
    from /opt/sensu/embedded/lib/ruby/gems/2.0.0/gems/sensu-0.20.1/bin/sensu-server:10:in `<top (required)>'
    from /opt/sensu/bin/sensu-server:23:in `load'
    from /opt/sensu/bin/sensu-server:23:in `<main>'

Looking at the code, the line it's complaining about is this:
if time_since_last_execution >= check[:ttl]

After a bunch of digging I found that someone had implemented a script to emit check results to port 3030 on a client, and the script was quoting the ttl value when it should have been unquoted.

I'm running v0.20.1 of sensu, but looking at the most recent code on github it appears this would happen in the most recent version.

Sensu server OR client should validate this value is actually an integer and log if not vs killing the sensu server.

skurylo pushed a commit to skurylo/sensu that referenced this issue Mar 4, 2016
fixes sensu#1182

If a user accidentally injects a string ttl value instead of an integer
it will crash the sensu server process.
@skurylo
Copy link

skurylo commented Mar 4, 2016

We recently ran into this issue. I've created a pull request with a possible solution.

The draw back of my solution is if the user supplies something like "foo" for the ttl, it will be changed to a zero.

I'm happy to implement a different solution, such ignoring the event or ttl and logging an error message.

@portertech
Copy link
Contributor

Result check TTL needs to be validated on input, i.e. https://github.com/sensu/sensu/blob/master/lib/sensu/client/socket.rb#L121

@portertech
Copy link
Contributor

Going to expedite this fix/feature for 0.23.2 👍

@portertech portertech removed this from the 0.24 milestone Apr 25, 2016
@portertech portertech self-assigned this Apr 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants