Skip to content

Commit

Permalink
Don't proxy #exec through #method_missing. 😢
Browse files Browse the repository at this point in the history
Redis has an EXEC command. We handle commands through `method_missing`.
This works great, normally:

  r = Redis::Namespace.new("foo")
  r.exec # => error, not in a transaction, whatever

Here's the problem: `Kernel#exec`. Since this is on every object, when
you use `#send`, it skips the normal visibility, and calls the private
but defined `Kernel#exec` rather than the `method_missing` verison:

  r = Redis::Namespace.new("foo")
  r.send(:exec, "ls") # => prints out your current directory

We fix this by not proxying `exec` through `method_missing`.

You are only vulnerable if you do the always-dangerous 'send random
input to an object through `send`.' And you probably don't. A cursory
search through GitHub didn't find anything that looked vulnerable.

However, if you write code that wraps or proxies Redis in some way, you
may do something like this.

The official Redis gem does not use `method_missing`, so it should not
have any issues:
https://github.com/redis/redis-rb/blob/master/lib/redis.rb#L2133-L2147

I plan on removing the `method_missing` implementation in a further
patch, but since this is an immediate issue, I wanted to make minimal
changes.

Testing this is hard, `#exec` replaces the current running process. I
have verified this fix by hand, but writing an automated test seems
difficult.

:heart: :cry:
  • Loading branch information
steveklabnik committed Aug 3, 2013
1 parent 5659711 commit 6d83951
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
4 changes: 4 additions & 0 deletions lib/redis/namespace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ def namespace(desired_namespace = nil)
@namespace
end

def exec

This comment has been minimized.

Copy link
@postmodern

postmodern Aug 4, 2013

You could also do undef exec.

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik Aug 4, 2013

Author Member

That's true, I guess. I think not doing method missing seems even better.

method_missing(:exec)
end

def method_missing(command, *args, &block)
handling = COMMANDS[command.to_s] ||
COMMANDS[ALIASES[command.to_s]]
Expand Down
2 changes: 1 addition & 1 deletion redis-namespace.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = "redis-namespace"
s.version = "1.3.0"
s.version = "1.3.1"
s.date = Time.now.strftime('%Y-%m-%d')
s.summary = "Namespaces Redis commands."
s.homepage = "http://github.com/resque/redis-namespace"
Expand Down

0 comments on commit 6d83951

Please sign in to comment.