Skip to content

Commit

Permalink
Fix SQL injection in Project API
Browse files Browse the repository at this point in the history
The query's where clause in project_get_all_user_rows() was built by
concatenating an unsanitized variable, allowing SQL injection via
SOAP API's mc_project_get_users() function using a crafted request.

Relying on DbQuery object ensures use of query parameters, making the
SQL injection impossible.

Partial backport from commit 682a182.

Fixes #27495, CVE-2020-28413
  • Loading branch information
dregad committed Dec 30, 2020
1 parent 30b3774 commit 3e37b40
Showing 1 changed file with 26 additions and 27 deletions.
53 changes: 26 additions & 27 deletions core/project_api.php
Expand Up @@ -665,42 +665,41 @@ function project_get_all_user_rows( $p_project_id = ALL_PROJECTS, $p_access_leve
}
}

if( is_array( $t_global_access_level ) ) {
if( 0 == count( $t_global_access_level ) ) {
$t_global_access_clause = '>= ' . NOBODY . ' ';
} else if( 1 == count( $t_global_access_level ) ) {
$t_global_access_clause = '= ' . array_shift( $t_global_access_level ) . ' ';
if( $p_include_global_users ) {
$t_query = new DbQuery();
$t_query->sql( 'SELECT id, username, realname, access_level
FROM {user}
WHERE enabled = ' . $t_query->param( $t_on ) . '
AND '
);
if( is_array( $t_global_access_level ) ) {
if( empty( $t_global_access_level ) ) {
$t_query->append_sql( 'access_level >= ' . $t_query->param( NOBODY ) );
} else {
$t_query->append_sql( $t_query->sql_in( 'access_level', $t_global_access_level ) );
}
} else {
$t_global_access_clause = 'IN (' . implode( ',', $t_global_access_level ) . ')';
$t_query->append_sql( 'access_level >= ' . $t_query->param( $t_global_access_level ) );
}
} else {
$t_global_access_clause = '>= ' . $t_global_access_level . ' ';
}
$t_query->execute();

if( $p_include_global_users ) {
db_param_push();
$t_query = 'SELECT id, username, realname, access_level
FROM {user}
WHERE enabled = ' . db_param() . '
AND access_level ' . $t_global_access_clause;
$t_result = db_query( $t_query, array( $t_on ) );

while( $t_row = db_fetch_array( $t_result ) ) {
while( $t_row = $t_query->fetch() ) {
$t_users[(int)$t_row['id']] = $t_row;
}
}

if( $c_project_id != ALL_PROJECTS ) {
# Get the project overrides
db_param_push();
$t_query = 'SELECT u.id, u.username, u.realname, l.access_level
FROM {project_user_list} l, {user} u
WHERE l.user_id = u.id
AND u.enabled = ' . db_param() . '
AND l.project_id = ' . db_param();
$t_result = db_query( $t_query, array( $t_on, $c_project_id ) );

while( $t_row = db_fetch_array( $t_result ) ) {
$t_query = new DbQuery();
$t_query->sql( 'SELECT u.id, u.username, u.realname, l.access_level
FROM {project_user_list} l, {user} u
WHERE l.user_id = u.id
AND u.enabled = ' . $t_query->param( $t_on ) . '
AND l.project_id = ' . $t_query->param( $c_project_id )
);
$t_query->execute();

while( $t_row = $t_query->fetch() ) {
if( is_array( $p_access_level ) ) {
$t_keep = in_array( $t_row['access_level'], $p_access_level );
} else {
Expand Down

0 comments on commit 3e37b40

Please sign in to comment.