Adamhumbug Posted July 7, 2023 Share Posted July 7, 2023 HI All, I am going through some old code and trying to sanitise sql injection problems and need some help on the best method here. The project does come with some caveats, which i cant change. I have to use Mysqli and cannot use PDO. Because the project is so large, i am going to need to deal with all code changes inside the function and not changing or removing other functions. I always appreciate the help on my personal stuff that i am doing and you guys are amazing but please on this one can we move past the fact that the code is rubbish and old and full of problems. I have a sql query that looks like this - as you can see there are variables in here as well as functions "select op_id, ".$opsrch['op_fullname'].", op_role from prs_op where ".$opsrch['srch'].(is_numeric($srch) ? " or op_id='".$srch."' or op_id like '".$srch."%'" : "")." order by ".$opsrch['sort']." limit 40"; I know how to write a prepared statement but i cannot do this simply as some of the variables contain more sql rather than just a simple variable. Once the sql and all of the variables and functions have been evaluated, this is what i get select op_id, CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) as op_fullname, op_role from prs_op where (CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like 'adam h%' or CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like 'adam h%' or CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like 'adam h%' or op_lastname like 'adam h%') order by op_firstname, op_lastname limit 40 All references to adam have been passed in from the front end. I have been looking at this -https://stackoverflow.com/questions/56135017/building-a-dynamic-php-prepared-statement-from-user-entry but that is putting everything in one place and im not sure if it can work here. I would really appreciate any help with info to tidy this mess - thanks in advance for helping me with crappy code. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 7, 2023 Share Posted July 7, 2023 (edited) in your example, does $opsrch['srch'] contain all that WHERE ... sql that also contains the $srch value and if so, where is $opsrch['srch'] built relative to the code you have posted? what php version will this be used with, because as of php8.1, you can simply supply an array of values to the ->execute([...]) call end eliminate all the explicit input binding and the type format string. all you would need to do is build an array with the input values that corresponds to the sql that is being built. edit: as long as all the values being a string data type are okay. i.e. won't work with a LIMIT clause which requires integers. If you are not using php8.1+, to convert this to a prepared query, using mysqli, you need to build the type format string and an array of input parameters that matches the sql that is being built. Edited July 7, 2023 by mac_gyver Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 7, 2023 Author Share Posted July 7, 2023 43 minutes ago, Adamhumbug said: HI All, I am going through some old code and trying to sanitise sql injection problems and need some help on the best method here. The project does come with some caveats, which i cant change. I have to use Mysqli and cannot use PDO. Because the project is so large, i am going to need to deal with all code changes inside the function and not changing or removing other functions. I always appreciate the help on my personal stuff that i am doing and you guys are amazing but please on this one can we move past the fact that the code is rubbish and old and full of problems. I have a sql query that looks like this - as you can see there are variables in here as well as functions "select op_id, ".$opsrch['op_fullname'].", op_role from prs_op where ".$opsrch['srch'].(is_numeric($srch) ? " or op_id='".$srch."' or op_id like '".$srch."%'" : "")." order by ".$opsrch['sort']." limit 40"; I know how to write a prepared statement but i cannot do this simply as some of the variables contain more sql rather than just a simple variable. Once the sql and all of the variables and functions have been evaluated, this is what i get select op_id, CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) as op_fullname, op_role from prs_op where (CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like 'adam h%' or CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like 'adam h%' or CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like 'adam h%' or op_lastname like 'adam h%') order by op_firstname, op_lastname limit 40 All references to adam have been passed in from the front end. I have been looking at this -https://stackoverflow.com/questions/56135017/building-a-dynamic-php-prepared-statement-from-user-entry but that is putting everything in one place and im not sure if it can work here. I would really appreciate any help with info to tidy this mess - thanks in advance for helping me with crappy code. If it is numeric i get this. select op_id, CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) as op_fullname, op_role from prs_op where (CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like '9999%' or CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like '9999%' or CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like '9999%' or op_lastname like '9999%') or op_id='9999' or op_id like '9999%' order by op_firstname, op_lastname limit 40 Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 7, 2023 Author Share Posted July 7, 2023 4 minutes ago, mac_gyver said: in your example, does $opsrch['srch'] contain all that WHERE ... sql that also contains the $srch value and if so, where is $opsrch['srch'] built relative to the code you have posted? what php version will this be used with, because as of php8.1, you can simply supply an array of values to the ->execute([...]) call end eliminate all the explicit input binding and the type format string. all you would need to do is build an array with the input values that corresponds to the sql that is being built. If you are not using php8.1+, to convert this to a prepared query, using mysqli, you need to build the type format string and an array of input parameters that matches the sql that is being built. $opsrch['srch'] contains "(CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like '".apos($sterm)."%' or CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like '".apos($sterm)."%' or CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like '".apos($sterm)."%' or op_lastname like '".apos($sterm)."%')"; Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 7, 2023 Author Share Posted July 7, 2023 6 minutes ago, mac_gyver said: in your example, does $opsrch['srch'] contain all that WHERE ... sql that also contains the $srch value and if so, where is $opsrch['srch'] built relative to the code you have posted? what php version will this be used with, because as of php8.1, you can simply supply an array of values to the ->execute([...]) call end eliminate all the explicit input binding and the type format string. all you would need to do is build an array with the input values that corresponds to the sql that is being built. If you are not using php8.1+, to convert this to a prepared query, using mysqli, you need to build the type format string and an array of input parameters that matches the sql that is being built. I believe that it is php 7.3. Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 7, 2023 Author Share Posted July 7, 2023 9 minutes ago, mac_gyver said: in your example, does $opsrch['srch'] contain all that WHERE ... sql that also contains the $srch value and if so, where is $opsrch['srch'] built relative to the code you have posted? what php version will this be used with, because as of php8.1, you can simply supply an array of values to the ->execute([...]) call end eliminate all the explicit input binding and the type format string. all you would need to do is build an array with the input values that corresponds to the sql that is being built. If you are not using php8.1+, to convert this to a prepared query, using mysqli, you need to build the type format string and an array of input parameters that matches the sql that is being built. My issue with this is that i will have to have several arrays as i am putting the code in several places in the query which i struggled to work out when referencing the stack overflow post i quoted. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 7, 2023 Share Posted July 7, 2023 array_merge() Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 7, 2023 Author Share Posted July 7, 2023 1 hour ago, mac_gyver said: array_merge() So the SQL would end up looking something like? "select op_id, ".$arr[0].", op_role from prs_op where ".$arr[1] $arr[2] $arr[3]." limit 40"; Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 7, 2023 Share Posted July 7, 2023 you can build the sql query syntax with the same usage as now, with the $opsrch elements. what would change when converting to a prepared query will involve putting ? place-holders in the sql query statement where each value is at, building a type format string, and an array of variables that correspond to the place-holders. if your code is structured so that you are building parts of the query separately, you would build separate type format strings and arrays of input variables for each section, then conditionally concatenate the separate parts of the type format string and conditionally use array_merge() to produce the complete array of inputs that are supplied to the bind_param() statement. another subtle detail, any wild-card characters must be concatenated with the value in the variable, since the bind_param() uses references to the variables. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 7, 2023 Share Posted July 7, 2023 it doesn't seem like when the search is for a numerical value, that the name columns should be searched? will any of the name columns ever start with a numerical value? based on the snippets of the implied problem, i would implement this like the following - $opsrch['op_fullname'] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) as op_fullname"; $opsrch['sort'] = "op_firstname, op_lastname"; // the search input from wherever it is coming from $srch = '9999'; // a numerical value $srch = 'adam h'; // a string value // since mysqli requires variables in the bind_param statement, produce the search with a trailing wild-card in its own variable $sterm = "$srch%"; $types = []; // array to hold the type format strings $params = []; // array to hold the input variables $where_terms = []; // array to hold the where terms if(!is_numeric($srch)) { // do a name search $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "op_lastname like ?"; } else { // do an id search $types[] = 'i'; $params[] = $srch; $where_terms[] = "op_id= ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "op_id like ?"; } // build the sql query $sql = "SELECT op_id, {$opsrch['op_fullname']}, op_role FROM prs_op WHERE ".implode(' OR ',$where_terms)." ORDER BY {$opsrch['sort']} LIMIT 40"; // examine the result echo $sql; echo '<br>'; echo implode($types); echo '<pre>'; print_r($params); Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 11, 2023 Author Share Posted July 11, 2023 This has worked for one of my queries but actually for the one in the example it has not. Your example would require that i re-write all of the output of all of the queries that come back from the other functions that are run. Is there a way for me to do this without having to manually evaluate the functions and then hard code what they produce for the prepared statements? Quote Link to comment Share on other sites More sharing options...
Adamhumbug Posted July 11, 2023 Author Share Posted July 11, 2023 Just now, Adamhumbug said: This has worked for one of my queries but actually for the one in the example it has not. Your example would require that i re-write all of the output of all of the queries that come back from the other functions that are run. Is there a way for me to do this without having to manually evaluate the functions and then hard code what they produce for the prepared statements? Sorry to be more clear about what i mean - the where_terms i dont really want to have to write out explicitly if there is a way around it. I dont know if there is a good way of doing this, especially as they include params that have been passed in from the front end. Can you think of another way or do you think i am beat? Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 11, 2023 Share Posted July 11, 2023 because you only posted a small snippet of the problem, you got a small snippet of a solution. if you want a complete top down solution, you will need to post all the code that demonstrates the extent and limits of this problem. at a minimum, the code for each part of the query will need to be re-written so that it produces the sql query syntax, with the prepare query place-holders in it, the type format string for that part of the query, and an array of the input parameters for that part of the query. you can then merge the dynamically built parts together to produce the compete set of data needed to prepare, bind the input parameters, and execute the query. all the cases where there is conditional logic inside the sql query. e.g. the ternary operator, will need to be rewritten so that the logic can conditionally merge the three pieces of data for each part of the query, together in the correct order. have you actually used the mysqli extension with a prepared query so that you know what the end result will need to be? Quote Link to comment Share on other sites More sharing options...
cyberRobot Posted July 13, 2023 Share Posted July 13, 2023 On 7/7/2023 at 8:51 AM, Adamhumbug said: I believe that it is php 7.3. Note that you can check with phpversion()https://www.php.net/manual/en/function.phpversion.php In case you're not aware, support for PHP 7.3 ended in December 2021.https://www.php.net/supported-versions.php Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.