fix SQL injection: use parameterized query in login-discuz.php by zhblue · Pull Request #1145 · zhblue/hustoj

zhblue · 2026-03-30T02:14:01Z

Fix SQL Injection in login-discuz.php

问题

include/login-discuz.php 第17行，$user_id 直接拼入 SQL 字符串：

$sql="select password,salt,username from ".$discuz_db.".uc_members where username='$user_id'";

修复

改用参数化查询：

$sql = "SELECT password, salt, username FROM {$discuz_db}.uc_members WHERE username = ?";
$result = pdo_query($sql, $user_id);

注：仅当 $OJ_LOGIN_MOD="discuz" 时生效，默认配置为 "hustoj" 不受影响。

- admin/problem_df_change.php: wrap hlist processing in braces and apply array_map intval - admin/contest_add.php: same fix for hlist bypass of intval protection

- Replace string interpolation '$user_id' in SQL with ? placeholder - Pass $user_id as bound parameter to pdo_query() - Fixes both the main query and the discuz_conn branch

Copilot

Pull request overview

This PR aims to mitigate SQL injection risks in the Discuz login integration and harden a couple of admin endpoints that build IN (...) clauses from request data.

Changes:

Switch Discuz user lookup in login-discuz.php to a parameterized query (username = ?).
Sanitize comma-separated problem ID lists in admin actions by casting elements to integers before building IN (...) SQL.
Minor refactor to guard parsing logic behind isset($_POST['hlist']) in problem_df_change.php.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
trunk/web/include/login-discuz.php	Uses a bound parameter for `username` to avoid SQL injection, but leaves a broken/unused legacy connection block.
trunk/web/admin/problem_df_change.php	Casts `hlist` elements to `int` before interpolating into `IN (...)`.
trunk/web/admin/contest_add.php	Casts `hlist` elements to `int` before interpolating into `IN (...)` for updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T02:17:29Z

trunk/web/include/login-discuz.php

 		if($discuz_conn){
 			mysql_select_db($discuz_db,$discuz_conn);
-			$result=pdo_query($sql,$discuz_conn);
+			$result=pdo_query($sql, $user_id);


Inside the $discuz_conn block, mysql_select_db() does not affect pdo_query() (which uses the global PDO connection), and the second pdo_query($sql, $user_id) result is never applied to $row. This makes the block redundant and can cause Discuz login to fail if mysql_connect() fails even though the PDO query succeeds. Consider removing the legacy mysql_* connection entirely and rely on the parameterized pdo_query() result (or introduce a separate PDO connection for Discuz and refresh $row from that query).

@copilot apply changes based on this feedback

Done in commit 9226766. Removed the entire legacy mysql_connect() / mysql_select_db() block along with the unused host/port/user/pass variables and the redundant second pdo_query() call. The function now relies solely on the single parameterized pdo_query() result, with a null guard added to $row before the password comparison.

@copilot apply changes based on this feedback

This was already applied in commit 9226766 — the legacy mysql_connect() / mysql_select_db() block and all unused connection variables have been removed. The function now relies solely on the single parameterized pdo_query() result with a null guard on $row.

Copilot · 2026-03-30T02:17:30Z

trunk/web/admin/problem_df_change.php

+    $pieces = explode(",",$plist);
+    $pieces = array_unique($pieces);
+    if($pieces[0]=="")unset($pieces[0]);
+    // 安全修复：每个ID元素强制intval，防止SQL注入
+    $plist = implode(",", array_map('intval', $pieces));


array_map('intval', $pieces) will convert empty elements (e.g. input like "1,,2" or trailing commas) to 0, which then ends up in the IN ($plist) clause. It would be safer to trim/filter out empty strings and (optionally) drop non-positive IDs before imploding, and then run array_unique on the normalized integers to avoid duplicates reappearing after casting.

Suggested change

$pieces = explode(",",$plist);

$pieces = array_unique($pieces);

if($pieces[0]=="")unset($pieces[0]);

// 安全修复：每个ID元素强制intval，防止SQL注入

$plist = implode(",", array_map('intval', $pieces));

$pieces = explode(",", $plist);

// Trim each element

$pieces = array_map('trim', $pieces);

// Keep only non-empty, digit-only, positive IDs

$pieces = array_filter($pieces, function ($v) {

return $v !== '' && ctype_digit($v) && (int)$v > 0;

});

// Normalize to integers and remove duplicates

$intPieces = array_unique(array_map('intval', $pieces), SORT_NUMERIC);

if (empty($intPieces)) {

$plist = '';

} else {

// 安全修复：每个ID元素强制intval，防止SQL注入

$plist = implode(",", $intPieces);

}

Copilot · 2026-03-30T02:17:30Z

trunk/web/admin/contest_add.php

+  $pieces = explode(",",$plist);
  $pieces = array_unique($pieces);
  if($pieces[0]=="") unset($pieces[0]);
-  $plist=implode(",",$pieces);
+  // 安全修复：每个ID元素强制intval，防止SQL注入
+  $plist = implode(",", array_map('intval', $pieces));


array_map('intval', $pieces) can turn empty list elements into 0 (e.g. "1000,,1002"), which changes behavior from a SQL syntax error to including 0 in the IN (...) list. Consider filtering out empty strings and removing 0/negative IDs before building $plist, and de-duplicating after integer normalization.

Copilot · 2026-03-30T02:17:30Z

trunk/web/admin/problem_df_change.php

+  if(isset($_POST['hlist'])){
+    $plist = trim($_POST['hlist']);
+    $pieces = explode(",",$plist);
+    $pieces = array_unique($pieces);
+    if($pieces[0]=="")unset($pieces[0]);
+    // 安全修复：每个ID元素强制intval，防止SQL注入
+    $plist = implode(",", array_map('intval', $pieces));
+  }


The PR title/description focus on include/login-discuz.php, but this PR also changes SQL-related input handling in admin pages (problem_df_change.php, contest_add.php). Please update the PR description/title to reflect these additional security fixes (or split into separate PRs) so reviewers can assess scope and risk accurately.

Agent-Logs-Url: https://github.com/zhblue/hustoj/sessions/b0b6be2d-d44e-47e0-8db5-33be5a142331 Co-authored-by: zhblue <[email protected]>

zhblue added 2 commits March 30, 2026 09:59

fix SQL injection: intval each element in hlist before SQL IN clause

2586c64

- admin/problem_df_change.php: wrap hlist processing in braces and apply array_map intval - admin/contest_add.php: same fix for hlist bypass of intval protection

fix SQL injection: use parameterized query in login-discuz.php

d25a959

- Replace string interpolation '$user_id' in SQL with ? placeholder - Pass $user_id as bound parameter to pdo_query() - Fixes both the main query and the discuz_conn branch

Copilot AI review requested due to automatic review settings March 30, 2026 02:14

Copilot started reviewing on behalf of zhblue March 30, 2026 02:14 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Copilot started work on behalf of zhblue March 30, 2026 02:51 View session

fix: remove legacy mysql_* connection block from login-discuz.php

9226766

Agent-Logs-Url: https://github.com/zhblue/hustoj/sessions/b0b6be2d-d44e-47e0-8db5-33be5a142331 Co-authored-by: zhblue <[email protected]>

Copilot finished work on behalf of zhblue March 30, 2026 02:53

Copilot started work on behalf of zhblue March 30, 2026 03:07 View session

Copilot finished work on behalf of zhblue March 30, 2026 03:08

Merge branch 'master' into fix-sql-injection-login-discuz

4420a67

zhblue merged commit 3122861 into master Mar 30, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix SQL injection: use parameterized query in login-discuz.php#1145

fix SQL injection: use parameterized query in login-discuz.php#1145
zhblue merged 4 commits intomasterfrom
fix-sql-injection-login-discuz

zhblue commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

zhblue Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

zhblue Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    $pieces = explode(",",$plist);
-    $pieces = array_unique($pieces);
-    if($pieces[0]=="")unset($pieces[0]);
-    // 安全修复：每个ID元素强制intval，防止SQL注入
-    $plist = implode(",", array_map('intval', $pieces));
+    $pieces = explode(",", $plist);
+    // Trim each element
+    $pieces = array_map('trim', $pieces);
+    // Keep only non-empty, digit-only, positive IDs
+    $pieces = array_filter($pieces, function ($v) {
+      return $v !== '' && ctype_digit($v) && (int)$v > 0;
+    });
+    // Normalize to integers and remove duplicates
+    $intPieces = array_unique(array_map('intval', $pieces), SORT_NUMERIC);
+    if (empty($intPieces)) {
+      $plist = '';
+    } else {
+      // 安全修复：每个ID元素强制intval，防止SQL注入
+      $plist = implode(",", $intPieces);
+    }

Conversation

zhblue commented Mar 30, 2026

Fix SQL Injection in login-discuz.php

问题

修复

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

zhblue Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

zhblue Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants