mantisbt-plugins / timetracking

Other
27 stars 43 forks source link

Replace occurences of deprecated `db_query` API #13

Closed dregad closed 7 years ago

dregad commented 10 years ago

In MantisBT 1.3, the db_query() function will be removed from the API (see mantisbt/mantisbt#128). The following occurences have been found in this plugin; they should be replaced by db_query_bound() calls

TimeTracking/core/timetracking_api.php:52:$result = db_query( $query );
TimeTracking/pages/delete_record.php:29:   $result_pull_timerecords = db_query($query_pull_timerecords);
TimeTracking/pages/delete_record.php:39:   db_query($query_delete);
TimeTracking/pages/add_record.php:54:   if(!db_query($query)){
TimeTracking/TimeTracking.php:79:       $result_pull_timerecords = db_query( $query_pull_timerecords );
TimeTracking/TimeTracking.php:84:       $result_pull_hours = db_query( $query_pull_hours );
dregad commented 10 years ago

It's worth mentioning that simply replacing the function call is not sufficient; to avoid risk of sql injection attacks, any inline query parameters should be replaced by calls to db_param(). For example:

$t_query = "SELECT * FROM $table WHERE id = '$p_id'";
db_query($t_query);

Would become

$t_query = "SELECT * FROM $table WHERE id = '" . db_param() . "'";
db_query_bound($t_query, array( $p_id ) );
epenet commented 7 years ago

@dregad what is the recommended way of doing this in Mantis v2?

Is this correct?

$t_query = 'INSERT INTO '.$t_table .' ( user, bug_id, expenditure_date, hours, timestamp, category, info ) VALUES ( '.db_param().','.db_param().','.db_param().','.db_param().','.db_param().','.db_param().','.db_param().')';

if(!db_query($t_query, array($t_user, $f_bug_id, $t_expend, $t_time_value, $t_now, $f_time_category, $f_time_info))){ trigger_error( ERROR_DB_QUERY_FAILED, ERROR ); }

dregad commented 7 years ago

@epenet this issue was generated by a script in all plugins in which use of db_query() was detected. At the time, the function was deprecated, to be replaced by db_query_bound(). The latter was subsequently renamed to db_query(), so the only thing you should check is whether the SQL queries are using db_param() as explained in https://github.com/mantisbt-plugins/timetracking/issues/13#issuecomment-33578256 above.

In addition, you should also call db_param_push() before starting to build a query.

It is not necessary to trigger a ERROR_DB_QUERY_FAILED error yourself, db_query() does that already if execution fails.

epenet commented 7 years ago

v2 merged in: PR #21

epenet commented 7 years ago

I suppose this needs to stay open for the v1 branch. If anyone wants to make the changes, I'll happily merge them in.

epenet commented 7 years ago

I attempted to implement in this branch, but I don't have a 1.3 setup to test against. https://github.com/mantisbt-plugins/timetracking/tree/patch-db_query_bound

cproensa commented 7 years ago

fixed for plugin version 1.1 using mantis 1.3 syntax