Refactor `Snowflake` to avoid brakeman sql injection warnings (#25879)

This commit is contained in:
Matt Jankowski 2023-07-12 04:44:58 -04:00 committed by GitHub
parent 6c5a2233a8
commit f831452037
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 63 deletions

View File

@ -79,29 +79,6 @@
], ],
"note": "" "note": ""
}, },
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "75fcd147b7611763ab6915faf8c5b0709e612b460f27c05c72d8b9bd0a6a77f8",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "lib/mastodon/snowflake.rb",
"line": 87,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "connection.execute(\"CREATE OR REPLACE FUNCTION timestamp_id(table_name text)\\nRETURNS bigint AS\\n$$\\n DECLARE\\n time_part bigint;\\n sequence_base bigint;\\n tail bigint;\\n BEGIN\\n time_part := (\\n -- Get the time in milliseconds\\n ((date_part('epoch', now()) * 1000))::bigint\\n -- And shift it over two bytes\\n << 16);\\n\\n sequence_base := (\\n 'x' ||\\n -- Take the first two bytes (four hex characters)\\n substr(\\n -- Of the MD5 hash of the data we documented\\n md5(table_name || '#{SecureRandom.hex(16)}' || time_part::text),\\n 1, 4\\n )\\n -- And turn it into a bigint\\n )::bit(16)::bigint;\\n\\n -- Finally, add our sequence number to our base, and chop\\n -- it to the last two bytes\\n tail := (\\n (sequence_base + nextval(table_name || '_id_seq'))\\n & 65535);\\n\\n -- Return the time part and the sequence part. OR appears\\n -- faster here than addition, but they're equivalent:\\n -- time_part has no trailing two bytes, and tail is only\\n -- the last two bytes.\\n RETURN time_part | tail;\\n END\\n$$ LANGUAGE plpgsql VOLATILE;\\n\")",
"render_path": null,
"location": {
"type": "method",
"class": "Mastodon::Snowflake",
"method": "define_timestamp_id"
},
"user_input": "SecureRandom.hex(16)",
"confidence": "Medium",
"cwe_id": [
89
],
"note": ""
},
{ {
"warning_type": "Denial of Service", "warning_type": "Denial of Service",
"warning_code": 76, "warning_code": 76,

View File

@ -64,46 +64,7 @@ module Mastodon::Snowflake
def define_timestamp_id def define_timestamp_id
return if already_defined? return if already_defined?
connection.execute(<<~SQL) connection.execute(sanitized_timestamp_id_sql)
CREATE OR REPLACE FUNCTION timestamp_id(table_name text)
RETURNS bigint AS
$$
DECLARE
time_part bigint;
sequence_base bigint;
tail bigint;
BEGIN
time_part := (
-- Get the time in milliseconds
((date_part('epoch', now()) * 1000))::bigint
-- And shift it over two bytes
<< 16);
sequence_base := (
'x' ||
-- Take the first two bytes (four hex characters)
substr(
-- Of the MD5 hash of the data we documented
md5(table_name || '#{SecureRandom.hex(16)}' || time_part::text),
1, 4
)
-- And turn it into a bigint
)::bit(16)::bigint;
-- Finally, add our sequence number to our base, and chop
-- it to the last two bytes
tail := (
(sequence_base + nextval(table_name || '_id_seq'))
& 65535);
-- Return the time part and the sequence part. OR appears
-- faster here than addition, but they're equivalent:
-- time_part has no trailing two bytes, and tail is only
-- the last two bytes.
RETURN time_part | tail;
END
$$ LANGUAGE plpgsql VOLATILE;
SQL
end end
def ensure_id_sequences_exist def ensure_id_sequences_exist
@ -153,6 +114,57 @@ module Mastodon::Snowflake
SQL SQL
end end
def sanitized_timestamp_id_sql
ActiveRecord::Base.sanitize_sql_array(timestamp_id_sql_array)
end
def timestamp_id_sql_array
[timestamp_id_sql_string, { random_string: SecureRandom.hex(16) }]
end
def timestamp_id_sql_string
<<~SQL
CREATE OR REPLACE FUNCTION timestamp_id(table_name text)
RETURNS bigint AS
$$
DECLARE
time_part bigint;
sequence_base bigint;
tail bigint;
BEGIN
time_part := (
-- Get the time in milliseconds
((date_part('epoch', now()) * 1000))::bigint
-- And shift it over two bytes
<< 16);
sequence_base := (
'x' ||
-- Take the first two bytes (four hex characters)
substr(
-- Of the MD5 hash of the data we documented
md5(table_name || :random_string || time_part::text),
1, 4
)
-- And turn it into a bigint
)::bit(16)::bigint;
-- Finally, add our sequence number to our base, and chop
-- it to the last two bytes
tail := (
(sequence_base + nextval(table_name || '_id_seq'))
& 65535);
-- Return the time part and the sequence part. OR appears
-- faster here than addition, but they're equivalent:
-- time_part has no trailing two bytes, and tail is only
-- the last two bytes.
RETURN time_part | tail;
END
$$ LANGUAGE plpgsql VOLATILE;
SQL
end
def connection def connection
ActiveRecord::Base.connection ActiveRecord::Base.connection
end end