Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revamp database escaping fixes #5178 #5180

Open
wants to merge 18 commits into
base: release-2.1
from
Open
Diff settings

Always

Just for now

Copy path View file
@@ -100,7 +100,7 @@ function smf_db_initiate($db_server, $db_name, $db_user, $db_passwd, $db_prefix,
if (empty($db_options['dont_select_db']) && !@mysqli_select_db($connection, $db_name) && empty($db_options['non_fatal']))
display_db_error();
mysqli_query($connection, 'SET SESSION sql_mode = \'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION\'');
mysqli_query($connection, 'SET SESSION sql_mode = \'ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION,NO_BACKSLASH_ESCAPES\'');
This conversation was marked as resolved by albertlast

This comment has been minimized.

Copy link
@Sesquipedalian

Sesquipedalian Feb 19, 2019

Member

For the sake of easier reading, could you change this to use double quotes around the overall string? Then it could look like this:
mysqli_query($connection, "SET SESSION sql_mode = 'ONLY_FULL_GROUP_BY,STRICT...

This comment has been minimized.

Copy link
@albertlast

albertlast Feb 19, 2019

Author Collaborator

Both are good idea,
will change it later on.

This comment has been minimized.

Copy link
@albertlast

albertlast Feb 19, 2019

Author Collaborator

To get a better understanding i split the modes

This comment has been minimized.

Copy link
@live627

live627 Feb 19, 2019

Contributor

So why aren't we using $smcFunc for this?

This comment has been minimized.

Copy link
@albertlast

albertlast Feb 20, 2019

Author Collaborator

First, Why we should?
Secondly, it doesn't work.

This comment has been minimized.

Copy link
@live627

live627 Feb 20, 2019

Contributor
  • it handles escaping things for us
  • no? It accepts a connection object as the third param

This comment has been minimized.

Copy link
@albertlast

albertlast Feb 20, 2019

Author Collaborator

ther is no escaping, you would use the raw method --> provide nothing
feel free to do a pr with this changes.

The result would be, that the mysql connection get unstabler and slower --> got no problem with this.

if (!empty($db_options['db_mb4']))
$smcFunc['db_mb4'] = (bool) $db_options['db_mb4'];
@@ -414,7 +414,7 @@ function smf_db_query($identifier, $db_string, $db_values = array(), $connection
$old_pos = 0;
$pos = -1;
// Remove the string escape for better runtime
$db_string_1 = str_replace('\\\'', '', $db_string);
$db_string_1 = str_replace('\'\'','',$db_string);
This conversation was marked as resolved by albertlast

This comment has been minimized.

Copy link
@Sesquipedalian

Sesquipedalian Feb 19, 2019

Member

For the sake of easier reading, could you change this one to use double quotes around the overall string, too?

while (true)
{
$pos = strpos($db_string_1, '\'', $pos + 1);
Copy path View file
@@ -2146,7 +2146,7 @@ VALUES ('smfVersion', '{$smf_version}'),
('tfa_mode', '1'),
('allow_expire_redirect', '1'),
('json_done', '1'),
('displayFields', '[{"col_name":"cust_icq","title":"ICQ","type":"text","order":"1","bbc":"0","placement":"1","enclose":"<a class=\\"icq\\" href=\\"\\/\\/www.icq.com\\/people\\/{INPUT}\\" target=\\"_blank\\" title=\\"ICQ - {INPUT}\\"><img src=\\"{DEFAULT_IMAGES_URL}\\/icq.png\\" alt=\\"ICQ - {INPUT}\\"><\\/a>","mlist":"0"},{"col_name":"cust_skype","title":"Skype","type":"text","order":"2","bbc":"0","placement":"1","enclose":"<a href=\\"skype:{INPUT}?call\\"><img src=\\"{DEFAULT_IMAGES_URL}\\/skype.png\\" alt=\\"{INPUT}\\" title=\\"{INPUT}\\" \\/><\\/a> ","mlist":"0"},{"col_name":"cust_loca","title":"Location","type":"text","order":"4","bbc":"0","placement":"0","enclose":"","mlist":"0"},{"col_name":"cust_gender","title":"Gender","type":"radio","order":"5","bbc":"0","placement":"1","enclose":"<span class=\\" main_icons gender_{KEY}\\" title=\\"{INPUT}\\"><\\/span>","mlist":"0","options":["None","Male","Female"]}]'),
('displayFields', '[{"col_name":"cust_icq","title":"ICQ","type":"text","order":"1","bbc":"0","placement":"1","enclose":"<a class=\"icq\" href=\"\/\/www.icq.com\/people\/{INPUT}\" target=\"_blank\" title=\"ICQ - {INPUT}\"><img src=\"{DEFAULT_IMAGES_URL}\/icq.png\" alt=\"ICQ - {INPUT}\"><\/a>","mlist":"0"},{"col_name":"cust_skype","title":"Skype","type":"text","order":"2","bbc":"0","placement":"1","enclose":"<a href=\"skype:{INPUT}?call\"><img src=\"{DEFAULT_IMAGES_URL}\/skype.png\" alt=\"{INPUT}\" title=\"{INPUT}\" \/><\/a> ","mlist":"0"},{"col_name":"cust_loca","title":"Location","type":"text","order":"4","bbc":"0","placement":"0","enclose":"","mlist":"0"},{"col_name":"cust_gender","title":"Gender","type":"radio","order":"5","bbc":"0","placement":"1","enclose":"<span class=\" main_icons gender_{KEY}\" title=\"{INPUT}\"><\/span>","mlist":"0","options":["None","Male","Female"]}]'),
('minimize_files', '1'),
('securityDisable_moderate', '1');

@@ -2171,7 +2171,7 @@ VALUES (':)', '{$default_smiley_smiley}', 0, 0),
(':P', '{$default_tongue_smiley}', 10, 0),
(':-[', '{$default_embarrassed_smiley}', 11, 0),
(':-X', '{$default_lips_sealed_smiley}', 12, 0),
(':-\\', '{$default_undecided_smiley}', 13, 0),
(':-\', '{$default_undecided_smiley}', 13, 0),
(':-*', '{$default_kiss_smiley}', 14, 0),
(':''(', '{$default_cry_smiley}', 15, 0),
('>:D', '{$default_evil_smiley}', 16, 1),
@@ -2694,7 +2694,7 @@ VALUES ('smfVersion', '{$smf_version}'),
('tfa_mode', '1'),
('allow_expire_redirect', '1'),
('json_done', '1'),
('displayFields', '[{"col_name":"cust_icq","title":"ICQ","type":"text","order":"1","bbc":"0","placement":"1","enclose":"<a class=\\"icq\\" href=\\"\\/\\/www.icq.com\\/people\\/{INPUT}\\" target=\\"_blank\\" title=\\"ICQ - {INPUT}\\"><img src=\\"{DEFAULT_IMAGES_URL}\\/icq.png\\" alt=\\"ICQ - {INPUT}\\"><\\/a>","mlist":"0"},{"col_name":"cust_skype","title":"Skype","type":"text","order":"2","bbc":"0","placement":"1","enclose":"<a href=\\"skype:{INPUT}?call\\"><img src=\\"{DEFAULT_IMAGES_URL}\\/skype.png\\" alt=\\"{INPUT}\\" title=\\"{INPUT}\\" \\/><\\/a> ","mlist":"0"},"mlist":"0"},{"col_name":"cust_loca","title":"Location","type":"text","order":"4","bbc":"0","placement":"0","enclose":"","mlist":"0"},{"col_name":"cust_gender","title":"Gender","type":"radio","order":"5","bbc":"0","placement":"1","enclose":"<span class=\\" main_icons gender_{KEY}\\" title=\\"{INPUT}\\"><\\/span>","mlist":"0",,"options":["None","Male","Female"]}]'),
('displayFields', '[{"col_name":"cust_icq","title":"ICQ","type":"text","order":"1","bbc":"0","placement":"1","enclose":"<a class=\"icq\" href=\"\/\/www.icq.com\/people\/{INPUT}\" target=\"_blank\" title=\"ICQ - {INPUT}\"><img src=\"{DEFAULT_IMAGES_URL}\/icq.png\" alt=\"ICQ - {INPUT}\"><\/a>","mlist":"0"},{"col_name":"cust_skype","title":"Skype","type":"text","order":"2","bbc":"0","placement":"1","enclose":"<a href=\"skype:{INPUT}?call\"><img src=\"{DEFAULT_IMAGES_URL}\/skype.png\" alt=\"{INPUT}\" title=\"{INPUT}\" \/><\/a> ","mlist":"0"},{"col_name":"cust_loca","title":"Location","type":"text","order":"4","bbc":"0","placement":"0","enclose":"","mlist":"0"},{"col_name":"cust_gender","title":"Gender","type":"radio","order":"5","bbc":"0","placement":"1","enclose":"<span class=\" main_icons gender_{KEY}\" title=\"{INPUT}\"><\/span>","mlist":"0",,"options":["None","Male","Female"]}]'),
('minimize_files', '1'),
('securityDisable_moderate', '1');
# --------------------------------------------------------
@@ -2718,7 +2718,7 @@ VALUES (':)', '{$default_smiley_smiley}', 0, 0),
(':P', '{$default_tongue_smiley}', 10, 0),
(':-[', '{$default_embarrassed_smiley}', 11, 0),
(':-X', '{$default_lips_sealed_smiley}', 12, 0),
(':-\\', '{$default_undecided_smiley}', 13, 0),
(':-\', '{$default_undecided_smiley}', 13, 0),
(':-*', '{$default_kiss_smiley}', 14, 0),
(':''(', '{$default_cry_smiley}', 15, 0),
('>:D', '{$default_evil_smiley}', 16, 1),
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.