fix SSRF: validate user_id before GitHub API URL in starred() by zhblue · Pull Request #1144 · zhblue/hustoj

zhblue · 2026-03-30T02:12:46Z

Fix SSRF in starred() function

问题

include/my_func.inc.php 的 starred() 函数将 $user_id 直接拼入 GitHub API URL，若将来调用方式变化导致外部输入进入，可构造内网地址（如 127.0.0.1:1536）进行 SSRF 探测。

修复

在拼入 URL 前加两层校验：

is_valid_user_name() 通用格式校验
preg_replace('/[^a-zA-Z0-9\-_]/', '', ...) 白名单过滤

if (!$user_id || !is_valid_user_name($user_id)) {
    return false;
}
$safe_user_id = preg_replace('/[^a-zA-Z0-9\-_]/', '', $user_id);
if ($safe_user_id === '' || $safe_user_id !== $user_id) {
    return false;
}
$url = "https://api.github.com/users/{$safe_user_id}/starred?per_page=100";

- 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

- Add is_valid_user_name() check before embedding user_id in URL - Apply preg_replace whitelist filter (a-z 0-9 - _) as second layer - Prevents injection of internal addresses or arbitrary URLs if starred() is ever called with unchecked user input

Copilot

Pull request overview

This PR tightens input handling around user-controlled identifiers to reduce the risk of unsafe URL construction in starred() and to harden admin-side comma-separated ID lists used in SQL IN (...) clauses.

Changes:

Add user_id validation/sanitization before building the GitHub API URL in starred().
Sanitize hlist ID lists in admin endpoints by coercing elements to integers before building SQL.
Minor formatting/structure updates around hlist parsing.

Reviewed changes

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

File	Description
trunk/web/include/my_func.inc.php	Adds `user_id` validation before constructing the GitHub API request URL in `starred()`.
trunk/web/admin/problem_df_change.php	Sanitizes posted `hlist` values by converting list elements to integers before SQL `IN (...)`.
trunk/web/admin/contest_add.php	Applies similar integer coercion for `hlist` parsing in the `problem2contest` flow.

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

Copilot · 2026-03-30T02:15:51Z

trunk/web/include/my_func.inc.php

+    if (!$user_id || !is_valid_user_name($user_id)) {
+        return false;
+    }
+    $safe_user_id = preg_replace('/[^a-zA-Z0-9\-_]/', '', $user_id);


The second whitelist check conflicts with is_valid_user_name(): is_valid_user_name() allows a leading '' (see its implementation), but preg_replace() strips it and then the strict equality check forces a false return. If leading '' is valid in this codebase, adjust the whitelist to allow it (or use a single consistent validator). If this is meant to validate GitHub usernames specifically, consider replacing both checks with a dedicated GitHub-username validation and/or rawurlencode() the path segment instead of mutating then comparing.

Suggested change

$safe_user_id = preg_replace('/[^a-zA-Z0-9\-_]/', '', $user_id);

$safe_user_id = preg_replace('/[^a-zA-Z0-9\-\*_]/', '', $user_id);

Copilot · 2026-03-30T02:15:52Z

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 coerce empty/non-numeric entries to 0, which can produce unintended IDs in the IN() list (e.g. input "1,,2" becomes "1,0,2"). Consider trimming each element, filtering out empty/non-numeric values, and dropping <=0 IDs before implode() so the SQL only targets intended problem_ids.

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);

// 去除每个元素两端空白

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

// 过滤掉空字符串、非纯数字或非正数的ID，避免产生0或非法ID

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

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

});

// 去重

$pieces = array_unique($pieces);

if (empty($pieces)) {

$plist = "";

} else {

// 现在$pieces中仅包含经过验证的正整数ID字符串

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

}

Copilot · 2026-03-30T02:15:52Z

trunk/web/admin/contest_add.php

  $plist = trim($_POST['hlist']);
-  $pieces = explode(",",$plist );
+  $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));


This branch reads $_POST['hlist'] unconditionally; if the form submits problem2contest without hlist, it will raise an undefined index notice and $plist parsing will behave unexpectedly. Please guard access with isset()/?? and apply the same trim/filter-before-intval approach so empty/non-numeric items don’t become 0 in the generated list.

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

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

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

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Merge branch 'master' into fix-ssrf-starred-function

37ea61f

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix SSRF: validate user_id before GitHub API URL in starred()#1144

fix SSRF: validate user_id before GitHub API URL in starred()#1144
zhblue merged 3 commits intomasterfrom
fix-ssrf-starred-function

zhblue commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

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!

2 participants

	$safe_user_id = preg_replace('/[^a-zA-Z0-9\-_]/', '', $user_id);
	$safe_user_id = preg_replace('/[^a-zA-Z0-9\-\*_]/', '', $user_id);

-    $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);
+    // 去除每个元素两端空白
+    $pieces = array_map('trim', $pieces);
+    // 过滤掉空字符串、非纯数字或非正数的ID，避免产生0或非法ID
+    $pieces = array_filter($pieces, function ($v) {
+      return $v !== '' && ctype_digit($v) && intval($v) > 0;
+    });
+    // 去重
+    $pieces = array_unique($pieces);
+    if (empty($pieces)) {
+      $plist = "";
+    } else {
+      // 现在$pieces中仅包含经过验证的正整数ID字符串
+      $plist = implode(",", $pieces);
+    }

Conversation

zhblue commented Mar 30, 2026

Fix SSRF in starred() function

问题

修复

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!

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!

2 participants