From fe0e410adcddced191f2b3ceec99faa6f6ee2b31 Mon Sep 17 00:00:00 2001 From: Daniel Kukula Date: Tue, 29 Oct 2024 18:53:31 +0100 Subject: [PATCH 1/3] dk-remove_comment_string_validation --- lib/postgrex.ex | 16 +++------------- lib/postgrex/protocol.ex | 2 +- test/query_test.exs | 6 +++--- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/lib/postgrex.ex b/lib/postgrex.ex index 6c0c47ac..c7eb2f96 100644 --- a/lib/postgrex.ex +++ b/lib/postgrex.ex @@ -67,10 +67,6 @@ defmodule Postgrex do @max_rows 500 @timeout 15_000 - @comment_validation_error Postgrex.Error.exception( - message: "`:comment` option cannot contain sequence \"*/\"" - ) - ### PUBLIC API ### @doc """ @@ -332,15 +328,9 @@ defmodule Postgrex do defp comment_not_present!(opts) do case Keyword.get(opts, :comment) do - nil -> - true - - comment when is_binary(comment) -> - if String.contains?(comment, "*/") do - raise @comment_validation_error - else - false - end + nil -> true + comment when is_binary(comment) -> false + comment when is_list(comment) -> false end end diff --git a/lib/postgrex/protocol.ex b/lib/postgrex/protocol.ex index 3d681e67..007234c7 100644 --- a/lib/postgrex/protocol.ex +++ b/lib/postgrex/protocol.ex @@ -1603,7 +1603,7 @@ defmodule Postgrex.Protocol do end defp parse_describe_comment_msgs(query, comment, tail) when is_binary(comment) do - statement = [query.statement, "/*", comment, "*/"] + statement = [query.statement, ";/*", comment, "*/"] query = %{query | statement: statement} parse_describe_msgs(query, tail) end diff --git a/test/query_test.exs b/test/query_test.exs index 376962d8..65b11fe2 100644 --- a/test/query_test.exs +++ b/test/query_test.exs @@ -1853,10 +1853,10 @@ defmodule QueryTest do test "comment", context do assert [[123]] = query("select 123", [], comment: "query comment goes here") + assert [[123]] = query("select 123", [], comment: "query comment goes here;") + %Postgrex.Error{postgres: error} = query("select 123", [], comment: "*/ select 456 --") - assert_raise Postgrex.Error, fn -> - query("select 123", [], comment: "*/ DROP TABLE 123 --") - end + assert error.message =~ "cannot insert multiple commands into a prepared statement" end @tag :big_binary From 3c948b1f5a9e3a9ca7a9d382c5d638752bee009b Mon Sep 17 00:00:00 2001 From: Daniel Kukula Date: Tue, 29 Oct 2024 19:07:05 +0100 Subject: [PATCH 2/3] update tests --- test/query_test.exs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/query_test.exs b/test/query_test.exs index 65b11fe2..3f59bfc9 100644 --- a/test/query_test.exs +++ b/test/query_test.exs @@ -1851,12 +1851,17 @@ defmodule QueryTest do assert [["1", "2"], ["3", "4"]] = query("COPY (VALUES (1, 2), (3, 4)) TO STDOUT", [], opts) end - test "comment", context do + test "comment is not interfering with the query", context do assert [[123]] = query("select 123", [], comment: "query comment goes here") - assert [[123]] = query("select 123", [], comment: "query comment goes here;") - %Postgrex.Error{postgres: error} = query("select 123", [], comment: "*/ select 456 --") + assert [[123]] = query("select 123;", [], comment: "query comment goes here") + end + test "comment does not allow for sql injection", context do + %Postgrex.Error{postgres: error} = query("select 123", [], comment: "*/ select 456 --") assert error.message =~ "cannot insert multiple commands into a prepared statement" + + %Postgrex.Error{postgres: error} = query("select 123", [], comment: "*/ where false --") + assert error.message =~ ~s'syntax error at or near "where"' end @tag :big_binary From d2e132da1ed81b841533f8e37b853618e50665ee Mon Sep 17 00:00:00 2001 From: Daniel Kukula Date: Tue, 29 Oct 2024 19:08:20 +0100 Subject: [PATCH 3/3] iolist test --- test/query_test.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/query_test.exs b/test/query_test.exs index 3f59bfc9..cdc69b79 100644 --- a/test/query_test.exs +++ b/test/query_test.exs @@ -1852,6 +1852,7 @@ defmodule QueryTest do end test "comment is not interfering with the query", context do + assert [[123]] = query("select 123", [], comment: ["query", "comment" | ["goes here"]]) assert [[123]] = query("select 123", [], comment: "query comment goes here") assert [[123]] = query("select 123;", [], comment: "query comment goes here") end