diff --git a/src/utils/sql.jl b/src/utils/sql.jl index 366e198..99f424f 100644 --- a/src/utils/sql.jl +++ b/src/utils/sql.jl @@ -93,6 +93,21 @@ function strip_quote(var) end end +""" + quote_string(value::AbstractString) + +Render `value` as a SQL single-quoted string literal, escaping any embedded +single quotes by doubling them (per the SQL standard). + +This prevents SQL injection when attacker-controlled string values (for +example, values derived from the OPA `input` document such as a subject id) +are interpolated into the generated `where` clause. Without escaping, a value +like `bob' or '1'='1` would terminate the literal and inject arbitrary SQL. +""" +function quote_string(value::AbstractString) + return string("'", replace(value, "'" => "''"), "'") +end + before(::SQLVisitor, _node) = nothing after(visitor::SQLVisitor, _node) = pop!(visitor.result_stack) @@ -131,7 +146,7 @@ function visit(visitor::SQLVisitor, scaler::AST.OPAScalarValue) result = (value === nothing) ? "null" : (value === true) ? "true" : (value === false) ? "false" : - isa(value, String) ? "'$value'" : + isa(value, String) ? quote_string(value) : string(value) push!(visitor.result_stack, result) return nothing diff --git a/test/runtests.jl b/test/runtests.jl index 3d0bfbd..9b4f43c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -12,6 +12,7 @@ import OpenPolicyAgent.ASTWalker.SQL: SQLVisitor, SQLCondition, UnconditionalInc include("test_data.jl") include("test_utils.jl") include("sql_translate.jl") +import .OPASQL import .OPASQL: translate # Check version and help output @@ -340,6 +341,51 @@ function runtests() end end +function test_sql_string_escaping() + schema_map = Dict("data" => "public") + table_map = Dict("reports" => "juliahub_reports") + + # Helper to translate a single scalar value to its SQL literal form. + function scalar_to_sql(value) + visitor = SQLVisitor(schema_map, table_map) + SQL.visit(visitor, AST.OPAScalarValue(value)) + return pop!(visitor.result_stack) + end + + # Benign strings are still single-quoted as before. + @test scalar_to_sql("bob") == "'bob'" + @test scalar_to_sql("public") == "'public'" + + # Embedded single quotes must be doubled so they cannot break out of the + # literal and inject SQL. + @test scalar_to_sql("O'Brien") == "'O''Brien'" + @test scalar_to_sql(raw"bob' or '1'='1") == "'bob'' or ''1''=''1'" + @test scalar_to_sql("'; DROP TABLE juliahub_reports; --") == + "'''; DROP TABLE juliahub_reports; --'" + + # Non-string scalars are unaffected. + @test scalar_to_sql(Int64(4)) == "4" + @test scalar_to_sql(true) == "true" + @test scalar_to_sql(false) == "false" + @test scalar_to_sql(nothing) == "null" + + # The escaped literal must stay a single, balanced SQL string literal: + # outer quotes plus an even number of interior quote characters. + for malicious in (raw"bob' or '1'='1", "O'Brien", "'; DROP TABLE x; --", "a''b") + sql = scalar_to_sql(malicious) + @test startswith(sql, "'") && endswith(sql, "'") + @test iseven(count(==('\''), sql)) + end + + # The standalone reference translator (test/sql_translate.jl) must escape + # identically, since it is used as the expected-output oracle. + @test OPASQL.to_sql(OPASQL.OPAScalarValue(raw"bob' or '1'='1")) == + "'bob'' or ''1''=''1'" +end + @testset "OpenPolicyAgent" begin + @testset "SQL string escaping" begin + test_sql_string_escaping() + end runtests() -end \ No newline at end of file +end diff --git a/test/sql_translate.jl b/test/sql_translate.jl index bff08c7..36a07f4 100644 --- a/test/sql_translate.jl +++ b/test/sql_translate.jl @@ -178,6 +178,10 @@ function strip_quote(var) end end +# Escape embedded single quotes (per the SQL standard) so string literals +# cannot be used to inject SQL. Mirrors `OpenPolicyAgent.ASTWalker.SQL.quote_string`. +quote_string(value::AbstractString) = string("'", replace(value, "'" => "''"), "'") + function to_sql(scaler::OPAScalarValue) value = scaler.value if value === nothing @@ -187,7 +191,7 @@ function to_sql(scaler::OPAScalarValue) elseif value === false return "false" elseif isa(value, String) - return "'$value'" + return quote_string(value) else return string(value) end