diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f8fa6d..2a0419b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ # Changes * 2.0.next in progress + * Since Alpha 3, more documentation has been written and existing documentation clarified (addressing #300, #309, #313, #314). * Fix #319 by ensuring `register-clause!` is idempotent. * Fix #317 by dropping qualifiers in `:set` clauses (just like we do with `:insert` columns). Note that you can still use explicit _dotted_ names if you want table qualification. + * Fix #316 by disallowing entity names containing `;` (to avoid SQL injection risks). * Fix #312 by adding `:raw` as a clause. There is no helper function equivalent (because it would be ambiguous whether you meant a function form -- `[:raw ..]` -- or a clause form -- `{:raw ..}`; and for the same reason, there is no `nest` helper function since that also works as a clause and as a function/special syntax). * 2.0.0-alpha3 (for early testing; 2021-03-13) @@ -21,9 +23,11 @@ * Reconcile `where` behavior with recent 1.0 changes (porting #283 to v2). * Fix #280 by adding `:escape` as special syntax for regular expression patterns. * Fix #277 by adding `:join-by`/`join-by` so that you can have multiple `JOIN`'s in a specific order. + * 2.0.0-alpha2 (for early testing) * Since Alpha 1, a lot more documentation has been written and docstrings have been added to most functions in `honey.sql.helpers`. * Numerous small improvements have been made to clauses and helpers around insert/upsert. + * 2.0.0-alpha1 (for early testing) * This is a complete rewrite/simplification of HoneySQL that provides just two namespaces: * `honey.sql` -- this is the primary API via the `format` function as well as the various extension points. diff --git a/doc/clause-reference.md b/doc/clause-reference.md index a51cc7c..a482ac4 100644 --- a/doc/clause-reference.md +++ b/doc/clause-reference.md @@ -428,6 +428,8 @@ user=> (sql/format '{insert-into (((transport t) (id, name)) {select (*) from (c ["INSERT INTO transport AS t (id, name) SELECT * FROM cars"] ``` +> Note: if you specify `:columns` for an `:insert-into` that also includes column names, you will get invalid SQL. Similarly, if you specify `:columns` when `:values` is based on hash maps, you will get invalid SQL. Since clauses are generated independently, there is no cross-checking performed if you provide an illegal combination of clauses. + ## update `:update` expects either a simple SQL entity (table name) diff --git a/src/honey/sql.cljc b/src/honey/sql.cljc index 5be39e2..d138306 100644 --- a/src/honey/sql.cljc +++ b/src/honey/sql.cljc @@ -98,6 +98,10 @@ (def ^:private ^:dynamic *quoted* nil) (def ^:private ^:dynamic *inline* nil) (def ^:private ^:dynamic *params* nil) +;; there is no way, currently, to enable suspicious characters +;; in entities; if someone complains about this check, an option +;; can be added to format to turn this on: +(def ^:private ^:dynamic *allow-suspicious-entities* false) ;; clause helpers @@ -164,12 +168,18 @@ (if aliased [nil (nn x)] (let [[t c] (str/split (nn x) #"\.")] - (if c [t c] [nil t]))))] - (cond->> c - (not= "*" c) - (q) - t - (str (q t) ".")))) + (if c [t c] [nil t])))) + entity (cond->> c + (not= "*" c) + (q) + t + (str (q t) ".")) + suspicious #";"] + (when-not *allow-suspicious-entities* + (when (re-find suspicious entity) + (throw (ex-info (str "suspicious character found in entity: " entity) + {:disallowed suspicious})))) + entity)) (comment (for [v [:foo-bar 'foo-bar "foo-bar" diff --git a/src/honey/sql/helpers.cljc b/src/honey/sql/helpers.cljc index 8f9b4d4..5a1832f 100644 --- a/src/honey/sql/helpers.cljc +++ b/src/honey/sql/helpers.cljc @@ -592,12 +592,26 @@ (defn where "Accepts one or more SQL expressions (conditions) and - combines them with AND: + combines them with AND (by default): (where [:= :status 0] [:<> :task \"backup\"]) + or: + (where :and [:= :status 0] [:<> :task \"backup\"]) Produces: WHERE (status = ?) AND (task <> ?) - Parameters: 0 \"backup\"" + Parameters: 0 \"backup\" + + For a single expression, the brackets can be omitted: + + (where := :status 0) ; same as (where [:= :status 0]) + + With multiple expressions, the conjunction may be + specified as a leading symbol: + + (where :or [:= :status 0] [:= :task \"stop\"]) + + Produces: WHERE (status = 0) OR (task = ?) + Parameters: 0 \"stop\"" [& exprs] (generic :where exprs)) @@ -615,12 +629,24 @@ (defn having "Like `where`, accepts one or more SQL expressions - (conditions) and combines them with AND: + (conditions) and combines them with AND (by default): (having [:> :count 0] [:<> :name nil]) + or: + (having :and [:> :count 0] [:<> :name nil]) Produces: HAVING (count > ?) AND (name IS NOT NULL) - Parameters: 0" + Parameters: 0 + + (having :> :count 0) + + Produces: HAVING count > ? + Parameters: 0 + + (having :or [:> :count 0] [:= :name \"\"]) + + Produces: HAVING (count > ?) OR (name = ?) + Parameters: 0 \"\"" [& exprs] (generic :having exprs)) diff --git a/test/honey/sql/helpers_test.cljc b/test/honey/sql/helpers_test.cljc index 21b2e38..8daa0eb 100644 --- a/test/honey/sql/helpers_test.cljc +++ b/test/honey/sql/helpers_test.cljc @@ -20,65 +20,126 @@ with-data]])) (deftest test-select - (let [m1 (-> (with [:cte (-> (select :*) - (from :example) - (where [:= :example-column 0]))]) - (select-distinct :f.* :b.baz :c.quux [:b.bla "bla-bla"] - :%now [[:raw "@x := 10"]]) - (from [:foo :f] [:baz :b]) - (join :draq [:= :f.b :draq.x]) - (left-join [:clod :c] [:= :f.a :c.d]) - (right-join :bock [:= :bock.z :c.e]) - (full-join :beck [:= :beck.x :c.y]) - (where [:or - [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] - [:and [:< 1 2] [:< 2 3]] - [:in :f.e [1 [:param :param2] 3]] - [:between :f.e 10 20]]) - (group-by :f.a) - (having [:< 0 :f.e]) - (order-by [:b.baz :desc] :c.quux [:f.a :nulls-first]) - (limit 50) - (offset 10)) - m2 {:with [[:cte {:select [:*] - :from [:example] - :where [:= :example-column 0]}]] - :select-distinct [:f.* :b.baz :c.quux [:b.bla "bla-bla"] - :%now [[:raw "@x := 10"]]] - :from [[:foo :f] [:baz :b]] - :join [:draq [:= :f.b :draq.x]] - :left-join [[:clod :c] [:= :f.a :c.d]] - :right-join [:bock [:= :bock.z :c.e]] - :full-join [:beck [:= :beck.x :c.y]] - :where [:or - [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] - [:and [:< 1 2] [:< 2 3]] - [:in :f.e [1 [:param :param2] 3]] - [:between :f.e 10 20]] - :group-by [:f.a] - :having [:< 0 :f.e] - :order-by [[:b.baz :desc] :c.quux [:f.a :nulls-first]] - :limit 50 - :offset 10}] - (testing "Various construction methods are consistent" - (is (= m1 m2))) - (testing "SQL data formats correctly" - (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ?" - 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] - (sql/format m1 {:params {:param1 "gabba" :param2 2}})))) - #?(:clj (testing "SQL data prints and reads correctly" - (is (= m1 (read-string (pr-str m1)))))) - #_(testing "SQL data formats correctly with alternate param naming" - (is (= (sql/format m1 {:params {:param1 "gabba" :param2 2}}) - ["WITH cte AS (SELECT * FROM example WHERE example_column = $1) SELECT DISTINCT f.*, b.baz, c.quux, b.bla \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = $2) AND (b.baz <> $3)) OR (($4 < $5) AND ($6 < $7)) OR (f.e IN ($8, $9, $10)) OR f.e BETWEEN $11 AND $12 GROUP BY f.a HAVING $13 < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT $14 OFFSET $15" - 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10]))) - (testing "Locking" - (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS `bla-bla`, NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ? LOCK IN SHARE MODE" - 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] - (sql/format (assoc m1 :lock [:in-share-mode]) - {:params {:param1 "gabba" :param2 2} - ;; to enable :lock - :dialect :mysql :quoted false})))))) + (testing "large helper expression" + (let [m1 (-> (with [:cte (-> (select :*) + (from :example) + (where [:= :example-column 0]))]) + (select-distinct :f.* :b.baz :c.quux [:b.bla "bla-bla"] + :%now [[:raw "@x := 10"]]) + (from [:foo :f] [:baz :b]) + (join :draq [:= :f.b :draq.x]) + (left-join [:clod :c] [:= :f.a :c.d]) + (right-join :bock [:= :bock.z :c.e]) + (full-join :beck [:= :beck.x :c.y]) + (where [:or + [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] + [:and [:< 1 2] [:< 2 3]] + [:in :f.e [1 [:param :param2] 3]] + [:between :f.e 10 20]]) + (group-by :f.a) + (having [:< 0 :f.e]) + (order-by [:b.baz :desc] :c.quux [:f.a :nulls-first]) + (limit 50) + (offset 10)) + m2 {:with [[:cte {:select [:*] + :from [:example] + :where [:= :example-column 0]}]] + :select-distinct [:f.* :b.baz :c.quux [:b.bla "bla-bla"] + :%now [[:raw "@x := 10"]]] + :from [[:foo :f] [:baz :b]] + :join [:draq [:= :f.b :draq.x]] + :left-join [[:clod :c] [:= :f.a :c.d]] + :right-join [:bock [:= :bock.z :c.e]] + :full-join [:beck [:= :beck.x :c.y]] + :where [:or + [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] + [:and [:< 1 2] [:< 2 3]] + [:in :f.e [1 [:param :param2] 3]] + [:between :f.e 10 20]] + :group-by [:f.a] + :having [:< 0 :f.e] + :order-by [[:b.baz :desc] :c.quux [:f.a :nulls-first]] + :limit 50 + :offset 10}] + (testing "Various construction methods are consistent" + (is (= m1 m2))) + (testing "SQL data formats correctly" + (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ?" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] + (sql/format m1 {:params {:param1 "gabba" :param2 2}})))) + #?(:clj (testing "SQL data prints and reads correctly" + (is (= m1 (read-string (pr-str m1)))))) + #_(testing "SQL data formats correctly with alternate param naming" + (is (= (sql/format m1 {:params {:param1 "gabba" :param2 2}}) + ["WITH cte AS (SELECT * FROM example WHERE example_column = $1) SELECT DISTINCT f.*, b.baz, c.quux, b.bla \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = $2) AND (b.baz <> $3)) OR (($4 < $5) AND ($6 < $7)) OR (f.e IN ($8, $9, $10)) OR f.e BETWEEN $11 AND $12 GROUP BY f.a HAVING $13 < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT $14 OFFSET $15" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10]))) + (testing "Locking" + (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS `bla-bla`, NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ? LOCK IN SHARE MODE" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] + (sql/format (assoc m1 :lock [:in-share-mode]) + {:params {:param1 "gabba" :param2 2} + ;; to enable :lock + :dialect :mysql :quoted false})))))) + (testing "large helper expression with simplified where" + (let [m1 (-> (with [:cte (-> (select :*) + (from :example) + (where := :example-column 0))]) + (select-distinct :f.* :b.baz :c.quux [:b.bla "bla-bla"] + :%now [[:raw "@x := 10"]]) + (from [:foo :f] [:baz :b]) + (join :draq [:= :f.b :draq.x]) + (left-join [:clod :c] [:= :f.a :c.d]) + (right-join :bock [:= :bock.z :c.e]) + (full-join :beck [:= :beck.x :c.y]) + (where :or + [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] + [:and [:< 1 2] [:< 2 3]] + [:in :f.e [1 [:param :param2] 3]] + [:between :f.e 10 20]) + (group-by :f.a) + (having :< 0 :f.e) + (order-by [:b.baz :desc] :c.quux [:f.a :nulls-first]) + (limit 50) + (offset 10)) + m2 {:with [[:cte {:select [:*] + :from [:example] + :where [:= :example-column 0]}]] + :select-distinct [:f.* :b.baz :c.quux [:b.bla "bla-bla"] + :%now [[:raw "@x := 10"]]] + :from [[:foo :f] [:baz :b]] + :join [:draq [:= :f.b :draq.x]] + :left-join [[:clod :c] [:= :f.a :c.d]] + :right-join [:bock [:= :bock.z :c.e]] + :full-join [:beck [:= :beck.x :c.y]] + :where [:or + [:and [:= :f.a "bort"] [:not= :b.baz :?param1]] + [:and [:< 1 2] [:< 2 3]] + [:in :f.e [1 [:param :param2] 3]] + [:between :f.e 10 20]] + :group-by [:f.a] + :having [:< 0 :f.e] + :order-by [[:b.baz :desc] :c.quux [:f.a :nulls-first]] + :limit 50 + :offset 10}] + (testing "Various construction methods are consistent" + (is (= m1 m2))) + (testing "SQL data formats correctly" + (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ?" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] + (sql/format m1 {:params {:param1 "gabba" :param2 2}})))) + #?(:clj (testing "SQL data prints and reads correctly" + (is (= m1 (read-string (pr-str m1)))))) + #_(testing "SQL data formats correctly with alternate param naming" + (is (= (sql/format m1 {:params {:param1 "gabba" :param2 2}}) + ["WITH cte AS (SELECT * FROM example WHERE example_column = $1) SELECT DISTINCT f.*, b.baz, c.quux, b.bla \"bla-bla\", NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = $2) AND (b.baz <> $3)) OR (($4 < $5) AND ($6 < $7)) OR (f.e IN ($8, $9, $10)) OR f.e BETWEEN $11 AND $12 GROUP BY f.a HAVING $13 < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT $14 OFFSET $15" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10]))) + (testing "Locking" + (is (= ["WITH cte AS (SELECT * FROM example WHERE example_column = ?) SELECT DISTINCT f.*, b.baz, c.quux, b.bla AS `bla-bla`, NOW(), @x := 10 FROM foo AS f, baz AS b INNER JOIN draq ON f.b = draq.x LEFT JOIN clod AS c ON f.a = c.d RIGHT JOIN bock ON bock.z = c.e FULL JOIN beck ON beck.x = c.y WHERE ((f.a = ?) AND (b.baz <> ?)) OR ((? < ?) AND (? < ?)) OR (f.e IN (?, ?, ?)) OR f.e BETWEEN ? AND ? GROUP BY f.a HAVING ? < f.e ORDER BY b.baz DESC, c.quux ASC, f.a NULLS FIRST LIMIT ? OFFSET ? LOCK IN SHARE MODE" + 0 "bort" "gabba" 1 2 2 3 1 2 3 10 20 0 50 10] + (sql/format (assoc m1 :lock [:in-share-mode]) + {:params {:param1 "gabba" :param2 2} + ;; to enable :lock + :dialect :mysql :quoted false}))))))) (deftest select-top-tests (testing "Basic TOP syntax" diff --git a/test/honey/sql_test.cljc b/test/honey/sql_test.cljc index 6b4d4ff..b7fe742 100644 --- a/test/honey/sql_test.cljc +++ b/test/honey/sql_test.cljc @@ -649,6 +649,20 @@ ORDER BY id = ? DESC :values [{:name name :enabled enabled}]}))))) +(deftest issue-316-test + (testing "SQL injection via keyword is detected" + (let [sort-column "foo; select * from users"] + (try + (-> {:select [:foo :bar] + :from [:mytable] + :order-by [(keyword sort-column)]} + (format)) + (is false "; not detected in entity!") + (catch #?(:clj Throwable :cljs :default) e + (is (:disallowed (ex-data e)))))) + ;; should not produce: ["SELECT foo, bar FROM mytable ORDER BY foo; select * from users"] + )) + (deftest issue-319-test (testing "that registering a clause is idempotent" (is (= ["FOO"]