Fix possible SQL injection issue (#18)

* Add a validation test case for the SQL injection

Signed-off-by: Mark Oude Veldhuis <mark.oudeveldhuis@nedap.com>

* Raise exception on invalid UUIDs

Signed-off-by: Mark Oude Veldhuis <mark.oudeveldhuis@nedap.com>

* Stay within 80 chars, add some comments.

Author: markoudev

lib/mysql-binuuid/type.rb

@@ -1,6 +1,7 @@
 module MySQLBinUUID
-  class Type < ActiveModel::Type::Binary
+  class InvalidUUID < StandardError; end
 
+  class Type < ActiveModel::Type::Binary
     def type
       :uuid
     end
@@ -27,7 +28,16 @@ def cast(value)
     # it to the database.
     def serialize(value)
       return if value.nil?
-      Data.new(strip_dashes(value))
+      undashed_uuid = strip_dashes(value)
+
+      # To avoid SQL injection, verify that it looks like a UUID. ActiveRecord
+      # does not explicity escape the Binary data type. escaping is implicit as
+      # the Binary data type always converts its value to a hex string.
+      unless valid_undashed_uuid?(undashed_uuid)
+        raise MySQLBinUUID::InvalidUUID, "#{value} is not a valid UUID"
+      end
+
+      Data.new(undashed_uuid)
     end
 
     # We're inheriting from the Binary type since ActiveRecord in that case
@@ -73,5 +83,10 @@ def strip_dashes(uuid)
       uuid.delete("-")
     end
 
+    # Verify that the undashed version of a UUID only contains characters that
+    # represent a hexadecimal value.
+    def valid_undashed_uuid?(value)
+      value =~ /\A[[:xdigit:]]{32}\z/
+    end
   end
 end

test/integration/mysql_integration_test.rb

@@ -112,6 +112,24 @@ class MyUuidModelWithValidations < MyUuidModel
       assert_equal results.first, @my_model
       assert_equal results.first.the_uuid, sample_uuid
     end
+
+    it "can't be used to inject SQL using .where" do
+      assert_raises MySQLBinUUID::InvalidUUID do
+        MyUuidModel.where(the_uuid: "' OR ''='").first
+      end
+    end
+
+    it "can't be used to inject SQL using .find_by" do
+      assert_raises MySQLBinUUID::InvalidUUID do
+        MyUuidModel.find_by(the_uuid: "' OR ''='")
+      end
+    end
+
+    it "can't be used to inject SQL while creating" do
+      assert_raises MySQLBinUUID::InvalidUUID do
+        MyUuidModel.create!(the_uuid: "40' + x'40")
+      end
+    end
   end
 
 end