Skip to content

Commit

Permalink
Prevent db errors for invalid bug/bugnote ids
Browse files Browse the repository at this point in the history
Some db engines may throw an error if provided with an integer out of
the supported range for field type. As an example: pgsql.
This can happen when the user provides a number high enough as input in
several places. For example:
- Bug and bugnote linking, in texts that are processed by core formatting
  plugin
- Bug jump quick access
- Bug id provided for a relation

The core formatting case is especially bad, because the bug page cannot
be rendered, always ending in a db error.

The fix is applied in bug_exists(), and bugnote_exists() functions,
which should fix most of said situations all through the applcation.

Fixes: #21802
  • Loading branch information
cproensa authored and vboctor committed Oct 23, 2016
1 parent 83fe03d commit 6c95495
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 21 deletions.
12 changes: 10 additions & 2 deletions core/bug_api.php
Expand Up @@ -985,10 +985,18 @@ function bug_text_clear_cache( $p_bug_id = null ) {
* @access public
*/
function bug_exists( $p_bug_id ) {
if( false == bug_cache_row( $p_bug_id, false ) ) {
$c_bug_id = (int)$p_bug_id;

# Check for invalid id values
if( $c_bug_id <= 0 || $c_bug_id > DB_MAX_INT ) {
return false;
} else {
}

# bug exists if bug_cache_row returns any value
if( bug_cache_row( $c_bug_id, false ) ) {
return true;
} else {
return false;
}
}

Expand Down
9 changes: 8 additions & 1 deletion core/bugnote_api.php
Expand Up @@ -128,9 +128,16 @@ class BugnoteData {
* @access public
*/
function bugnote_exists( $p_bugnote_id ) {
$c_bugnote_id = (int)$p_bugnote_id;

# Check for invalid id values
if( $c_bugnote_id <= 0 || $c_bugnote_id > DB_MAX_INT ) {
return false;
}

db_param_push();
$t_query = 'SELECT COUNT(*) FROM {bugnote} WHERE id=' . db_param();
$t_result = db_query( $t_query, array( $p_bugnote_id ) );
$t_result = db_query( $t_query, array( $c_bugnote_id ) );

if( 0 == db_result( $t_result ) ) {
return false;
Expand Down
8 changes: 7 additions & 1 deletion core/constant_inc.php
Expand Up @@ -617,4 +617,10 @@

# Maximum number of bugs that are treated simutaneously in export procedures,
# to keep memory usage under control. Do not exceed 1000 if using Oracle DB.
define( 'EXPORT_BLOCK_SIZE', 500 );
define( 'EXPORT_BLOCK_SIZE', 500 );

# Maximum "safe" value to be used for integer fields in database.
# Note: mantis ids are defined in schema as "I UNSIGNED", which Adodb maps to
# the closest integer (4 bytes) type available. As some DBs dont support unsigned
# types, 2^31 is a safe limit to be used for all.
define( 'DB_MAX_INT', 2147483647 );
39 changes: 22 additions & 17 deletions core/string_api.php
Expand Up @@ -347,13 +347,14 @@ function string_process_bug_link( $p_string, $p_include_anchor = true, $p_detail
if( $p_include_anchor ) {
$s_bug_link_callback[$p_include_anchor][$p_detail_info][$p_fqdn] =
function( $p_array ) use( $p_detail_info, $p_fqdn ) {
if( bug_exists( (int)$p_array[2] ) ) {
$t_project_id = bug_get_field( (int)$p_array[2], 'project_id' );
$c_bug_id = (int)$p_array[2];
if( bug_exists( $c_bug_id ) ) {
$t_project_id = bug_get_field( $c_bug_id, 'project_id' );
$t_view_bug_threshold = config_get( 'view_bug_threshold', null, null, $t_project_id );
if( access_has_bug_level( $t_view_bug_threshold, (int)$p_array[2] ) ) {
if( access_has_bug_level( $t_view_bug_threshold, $c_bug_id ) ) {
return $p_array[1] .
string_get_bug_view_link(
(int)$p_array[2],
$c_bug_id,
(boolean)$p_detail_info,
(boolean)$p_fqdn
);
Expand All @@ -364,10 +365,11 @@ function( $p_array ) use( $p_detail_info, $p_fqdn ) {
} else {
$s_bug_link_callback[$p_include_anchor][$p_detail_info][$p_fqdn] =
function( $p_array ) {
if( bug_exists( (int)$p_array[2] ) ) {
$c_bug_id = (int)$p_array[2];
if( bug_exists( $c_bug_id ) ) {
# Create link regardless of user's access to the bug
return $p_array[1] .
string_get_bug_view_url_with_fqdn( (int)$p_array[2] );
string_get_bug_view_url_with_fqdn( $c_bug_id );
}
return $p_array[0];
}; # end of bug link callback closure
Expand Down Expand Up @@ -416,23 +418,24 @@ function string_process_bugnote_link( $p_string, $p_include_anchor = true, $p_de
$s_bugnote_link_callback[$p_include_anchor][$p_detail_info][$p_fqdn] =
function( $p_array ) use( $p_detail_info, $p_fqdn ) {
global $g_project_override;
if( bugnote_exists( (int)$p_array[2] ) ) {
$t_bug_id = bugnote_get_field( (int)$p_array[2], 'bug_id' );
$c_bugnote_id = (int)$p_array[2];
if( bugnote_exists( $c_bugnote_id ) ) {
$t_bug_id = bugnote_get_field( $c_bugnote_id, 'bug_id' );
if( bug_exists( $t_bug_id ) ) {
$g_project_override = bug_get_field( $t_bug_id, 'project_id' );
if( access_compare_level(
user_get_access_level( auth_get_current_user_id(),
bug_get_field( $t_bug_id, 'project_id' ) ),
config_get( 'private_bugnote_threshold' )
)
|| bugnote_get_field( (int)$p_array[2], 'reporter_id' ) == auth_get_current_user_id()
|| bugnote_get_field( (int)$p_array[2], 'view_state' ) == VS_PUBLIC
|| bugnote_get_field( $c_bugnote_id, 'reporter_id' ) == auth_get_current_user_id()
|| bugnote_get_field( $c_bugnote_id, 'view_state' ) == VS_PUBLIC
) {
$g_project_override = null;
return $p_array[1] .
string_get_bugnote_view_link(
$t_bug_id,
(int)$p_array[2],
$c_bugnote_id,
(boolean)$p_detail_info,
(boolean)$p_fqdn
);
Expand All @@ -445,13 +448,15 @@ function( $p_array ) use( $p_detail_info, $p_fqdn ) {
} else {
$s_bugnote_link_callback[$p_include_anchor][$p_detail_info][$p_fqdn] =
function( $p_array ) {
$t_bug_id = bugnote_get_field( (int)$p_array[2], 'bug_id' );
if( $t_bug_id && bug_exists( $t_bug_id ) ) {
return $p_array[1] .
string_get_bugnote_view_url_with_fqdn( $t_bug_id, (int)$p_array[2] );
} else {
return $p_array[0];
$c_bugnote_id = (int)$p_array[2];
if( bugnote_exists( $c_bugnote_id ) ) {
$t_bug_id = bugnote_get_field( $c_bugnote_id, 'bug_id' );
if( $t_bug_id && bug_exists( $t_bug_id ) ) {
return $p_array[1] .
string_get_bugnote_view_url_with_fqdn( $t_bug_id, $c_bugnote_id );
}
}
return $p_array[0];
}; # end of bugnote link callback closure
}
}
Expand Down

0 comments on commit 6c95495

Please sign in to comment.