From 08e858d185771572d8497ec502d42dea6ea651ce Mon Sep 17 00:00:00 2001 From: Davide Paolo Tua Date: Wed, 22 May 2024 17:24:34 +0200 Subject: [PATCH 1/2] manage raw where clause with array as param --- spec/avram/query_builder_spec.cr | 50 +++++++++++++++++++++++++++----- src/avram/where.cr | 24 +++++++++++---- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/spec/avram/query_builder_spec.cr b/spec/avram/query_builder_spec.cr index b74dc5425..4894784bf 100644 --- a/spec/avram/query_builder_spec.cr +++ b/spec/avram/query_builder_spec.cr @@ -95,14 +95,48 @@ describe Avram::QueryBuilder do query.statement.should eq "SELECT * FROM users WHERE name = $1 AND age > $2" end - it "accepts raw where clauses" do - query = new_query - .where(Avram::Where::Raw.new("name = ?", "Mikias")) - .where(Avram::Where::Raw.new("age > ?", 26)) - .where(Avram::Where::Raw.new("age < ?", args: [30])) - .limit(1) - query.statement.should eq "SELECT * FROM users WHERE name = 'Mikias' AND age > 26 AND age < 30 LIMIT 1" - query.args.empty?.should be_true + describe "accepts raw clauses" do + it "substituting binding parameters" do + query = new_query + .where(Avram::Where::Raw.new("name = ?", "Mikias")) + .where(Avram::Where::Raw.new("age > ?", 26)) + .where(Avram::Where::Raw.new("age < ?", args: [30])) + .limit(1) + query.statement.should eq "SELECT * FROM users WHERE name = 'Mikias' AND age > 26 AND age < 30 LIMIT 1" + query.args.empty?.should be_true + end + + it "escaping elements to prevent sql injections" do + expected = <<-SQL + SELECT * FROM users WHERE name = 'aloha'';--' + SQL + query = new_query.where(Avram::Where::Raw.new("name = ? ", "aloha';--")) + query.statement.should eq expected + end + + it "correctly managing input arrays" do + expected = <<-SQL + SELECT * FROM users WHERE tags && '{"ruby","crystal"}' + SQL + query = new_query.where(Avram::Where::Raw.new("tags && ?", ["ruby", "crystal"])) + query.statement.should eq expected + end + + it "preventing sql injections with arrays (1)" do + expected = <<-SQL + SELECT * FROM users WHERE tags && '{"ruby","crystal';--"}' + SQL + query = new_query.where(Avram::Where::Raw.new("tags && ? ", ["ruby", "crystal';--"])) + query.statement.should eq expected + end + + it "preventing sql injections with arrays (2)" do + expected = <<-SQL + SELECT * FROM users WHERE tags && '{"ruby","crystal\\"}';--"}' + SQL + query = new_query.where(Avram::Where::Raw.new("tags && ? ", ["ruby", "crystal\"}';--"])) + query.statement.should eq expected + end end it "can be ordered" do diff --git a/src/avram/where.cr b/src/avram/where.cr index c64b92090..09b26a744 100644 --- a/src/avram/where.cr +++ b/src/avram/where.cr @@ -446,14 +446,26 @@ module Avram::Where private def build_clause(statement, bind_vars) bind_vars.each do |arg| - if arg.is_a?(String) || arg.is_a?(Slice(UInt8)) - escaped = PG::EscapeHelper.escape_literal(arg) - else - escaped = arg - end - statement = statement.sub('?', escaped) + encoded_arg = prepare_for_execution(arg) + statement = statement.sub('?', encoded_arg) end statement end + + private def prepare_for_execution(value) + if value.is_a?(Array) + "'#{PQ::Param.encode_array(value)}'" + else + escape_if_needed(value) + end + end + + private def escape_if_needed(value) + if value.is_a?(String) || value.is_a?(Slice(UInt8)) + PG::EscapeHelper.escape_literal(value) + else + value + end + end end end From 8ba499d6a5cad9844f5bb4100f310f6f407ccf47 Mon Sep 17 00:00:00 2001 From: Davide Paolo Tua Date: Wed, 22 May 2024 18:04:02 +0200 Subject: [PATCH 2/2] ameba fun... --- spec/avram/query_builder_spec.cr | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/avram/query_builder_spec.cr b/spec/avram/query_builder_spec.cr index 4894784bf..a9ae98a72 100644 --- a/spec/avram/query_builder_spec.cr +++ b/spec/avram/query_builder_spec.cr @@ -108,9 +108,9 @@ describe Avram::QueryBuilder do it "escaping elements to prevent sql injections" do expected = <<-SQL - SELECT * FROM users WHERE name = 'aloha'';--' + SELECT * FROM users WHERE name = 'aloha'';--' SQL - query = new_query.where(Avram::Where::Raw.new("name = ? ", "aloha';--")) + query = new_query.where(Avram::Where::Raw.new("name = ?", "aloha';--")) query.statement.should eq expected end @@ -124,17 +124,17 @@ describe Avram::QueryBuilder do it "preventing sql injections with arrays (1)" do expected = <<-SQL - SELECT * FROM users WHERE tags && '{"ruby","crystal';--"}' + SELECT * FROM users WHERE tags && '{"ruby","crystal';--"}' SQL - query = new_query.where(Avram::Where::Raw.new("tags && ? ", ["ruby", "crystal';--"])) + query = new_query.where(Avram::Where::Raw.new("tags && ?", ["ruby", "crystal';--"])) query.statement.should eq expected end it "preventing sql injections with arrays (2)" do expected = <<-SQL - SELECT * FROM users WHERE tags && '{"ruby","crystal\\"}';--"}' + SELECT * FROM users WHERE tags && '{"ruby","crystal\\"}';--"}' SQL - query = new_query.where(Avram::Where::Raw.new("tags && ? ", ["ruby", "crystal\"}';--"])) + query = new_query.where(Avram::Where::Raw.new("tags && ?", ["ruby", "crystal\"}';--"])) query.statement.should eq expected end end