fix XSS: add htmlentities to all direct $_SESSION echo outputs#1142
Merged
fix XSS: add htmlentities to all direct $_SESSION echo outputs#1142
Conversation
- template/*: sidebar, bs3, bshark, sweet, mdui - header, submitpage, status, conteststatus, problem, problemset, modifypage, problem1, problem2 - admin/: contest_list, news_list, phpfm, problem_list, rejudge, privilege_add, user_list - root: thread, newpost, export_ac_code, include/set_post_key - Fixed getkey parameters and user_id outputs in JS strings, HTML text, URL params - csrf.php class attribute output (low risk, boolean)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the PHP frontend against XSS by consistently escaping direct $_SESSION[...] outputs across multiple templates and admin pages.
Changes:
- Wrapped various
$_SESSION-sourced values (e.g.,user_id,getkey,postkey) withhtmlentities(..., ENT_QUOTES, 'UTF-8')before rendering. - Updated multiple theme templates (sidebar/bs3/bshark/sweet/mdui) to escape session-derived values in HTML and embedded JS.
- Updated several admin pages and helper includes to escape session-derived values used in links/forms.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| trunk/web/thread.php | Escape session user_id when rendering “reply as” user link |
| trunk/web/template/sweet/problem.php | Escape session getkey in admin edit link |
| trunk/web/template/sweet/modifypage.php | Escape displayed session user_id |
| trunk/web/template/sidebar/submitpage.php | Escape session user_id used in autosave localStorage key |
| trunk/web/template/sidebar/status.php | Escape session user_id injected into JS |
| trunk/web/template/sidebar/problemset.php | Escape session getkey in admin links |
| trunk/web/template/sidebar/problem2.php | Escape session getkey in edit links |
| trunk/web/template/sidebar/problem1.php | Escape session getkey in edit links |
| trunk/web/template/sidebar/problem.php | Escape session getkey in edit link |
| trunk/web/template/sidebar/modifypage.php | Escape session user_id in disabled input |
| trunk/web/template/sidebar/header.php | Escape session user_id/nick/group_name in header dropdown + SaaS link |
| trunk/web/template/sidebar/conteststatus.php | Escape session user_id injected into JS |
| trunk/web/template/mdui/problem.php | Escape session getkey in admin edit link |
| trunk/web/template/mdui/modifypage.php | Escape session user_id in disabled input |
| trunk/web/template/mdui/_includes/header.php | Escape session user_id in header |
| trunk/web/template/bshark/submitpage.php | Escape session user_id used in autosave localStorage key |
| trunk/web/template/bshark/problem.php | Escape session getkey in edit link |
| trunk/web/template/bs3/submitpage.php | Escape session user_id used in autosave localStorage key |
| trunk/web/template/bs3/status.php | Escape session user_id injected into JS |
| trunk/web/template/bs3/problem.php | Escape session getkey in edit link |
| trunk/web/template/bs3/conteststatus.php | Escape session user_id injected into JS |
| trunk/web/newpost.php | Escape session user_id when rendering author link |
| trunk/web/include/set_post_key.php | Escape session postkey when rendering hidden input |
| trunk/web/export_ac_code.php | Escape session user_id when outputting export header line |
| trunk/web/admin/user_list.php | Escape session getkey in action links |
| trunk/web/admin/rejudge.php | Escape session postkey in hidden inputs |
| trunk/web/admin/problem_list.php | Escape session getkey in action links |
| trunk/web/admin/privilege_add.php | Escape session postkey in hidden inputs |
| trunk/web/admin/phpfm.php | Escape session administrator value in unauthorized branch |
| trunk/web/admin/news_list.php | Escape session getkey in defunct toggle link |
| trunk/web/admin/contest_list.php | Escape session getkey in admin links |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <?php if ($OJ_SaaS_ENABLE){ ?> | ||
| <?php if($_SERVER['HTTP_HOST']==$DOMAIN) | ||
| echo "<a class='item' href='http://". $_SESSION[$OJ_NAME.'_'.'user_id'].".$DOMAIN'><i class='globe icon' ></i>MyOJ</a>";?> | ||
| echo "<a class='item' href='http://".htmlentities($_SESSION[$OJ_NAME.'_'.'user_id'], ENT_QUOTES, 'UTF-8']).".$DOMAIN'><i class='globe icon' ></i>MyOJ</a>";?> |
There was a problem hiding this comment.
Line has a PHP syntax error due to an extra closing bracket in the htmlentities call, which will break rendering of the header/menu. Remove the stray ']' so the string concatenation is valid.
| ||isset($_SESSION[$OJ_NAME.'_'.'problem_editor']) | ||
| )){ | ||
| echo $_SESSION[$OJ_NAME.'_'.'administrator']; | ||
| echo htmlentities($_SESSION[$OJ_NAME.'_'.'administrator'], ENT_QUOTES, 'UTF-8'); |
There was a problem hiding this comment.
This echo calls $SESSION[$OJ_NAME.''.'administrator'] in the unauthorized branch without checking it exists. When the session key is not set, this triggers an undefined index notice and may leak debug output; consider removing this echo or guarding it with isset().
Suggested change
| echo htmlentities($_SESSION[$OJ_NAME.'_'.'administrator'], ENT_QUOTES, 'UTF-8'); |
| $redis = new Redis(); | ||
| $redis->connect($OJ_REDISSERVER, $OJ_REDISPORT); | ||
| if(isset($OJ_REDISAUTH)) $redis->auth($OJ_REDISAUTH); | ||
| $sql="select solution_id from solution where result=1,pass_rate=0 and problem_id>0"; |
There was a problem hiding this comment.
SQL uses a comma in the WHERE clause (result=1,pass_rate=0), which is invalid SQL and will cause pdo_query() to fail when OJ_REDIS is enabled. Replace the comma with AND to correctly filter on both columns.
Suggested change
| $sql="select solution_id from solution where result=1,pass_rate=0 and problem_id>0"; | |
| $sql="select solution_id from solution where result=1 and pass_rate=0 and problem_id>0"; |
| if(isset($_SESSION[$OJ_NAME.'_'.'administrator'])||isset($_SESSION[$OJ_NAME.'_'.'problem_editor'])){ | ||
| if(isset($_SESSION[$OJ_NAME.'_'.'administrator']) || isset($_SESSION[$OJ_NAME.'_'.'problem_editor'])){ | ||
| echo "<td><a href=problem_df_change.php?id=".$row['problem_id']."&getkey=".$_SESSION[$OJ_NAME.'_'.'getkey'].">".($row['defunct']=="N"?"<span titlc='click to reserve it' class=green>$MSG_AVAILABLE</span>":"<span class=red title='click to be available'>$MSG_RESERVED</span>")."</a><td>"; | ||
| echo "<td><a href=problem_df_change.php?id=".$row['problem_id']."&getkey=".htmlentities($_SESSION[$OJ_NAME.'_'.'getkey'], ENT_QUOTES, 'UTF-8').">".($row['defunct']=="N"?"<span titlc='click to reserve it' class=green>$MSG_AVAILABLE</span>":"<span class=red title='click to be available'>$MSG_RESERVED</span>")."</a><td>"; |
There was a problem hiding this comment.
This table cell is opened with <td> but closed with another <td> (...</a><td>), producing invalid HTML and potentially breaking the table layout. Close it with </td> instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix XSS: add htmlentities to all direct $_SESSION echo outputs
问题
多处模板文件直接 echo $_SESSION 变量，未做 htmlentities 转义，存在 XSS 风险。
修复内容
修复方式
统一使用
htmlentities($_SESSION[xxx], ENT_QUOTES, UTF-8)进行转义