Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates judge_client.cc to tighten cleanup behavior around work_dir, fix an incorrect strstr check when handling NOIP output filenames, and adjust ARM64 register debug printing.
Changes:
- Adjust ARM64 register debug output formatting in
print_arm_regs. - Add basic
work_dir/userfilevalidation gates before runningrmdir/rmshell commands. - Fix
strstrto check the actualnoip_file_namevalue rather than a string literal, and add an early-return guard inclean_workdir.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void print_arm_regs(long long unsigned int *d){ | ||
| for(int i=0;i<32;i++){ | ||
| printf("[%d]:%llu ",i,d[i]%CALL_ARRAY_SIZE); | ||
| printf("[%d]:%lld ",i,d[i]%CALL_ARRAY_SIZE); |
There was a problem hiding this comment.
print_arm_regs takes long long unsigned int* but uses %lld to print d[i] % CALL_ARRAY_SIZE. This is a format/argument type mismatch (should be unsigned, e.g. %llu or PRIu64 after casting) and can print incorrect values / trigger UB on some platforms.
| printf("[%d]:%lld ",i,d[i]%CALL_ARRAY_SIZE); | |
| printf("[%d]:%llu ", i, (unsigned long long)(d[i] % CALL_ARRAY_SIZE)); |
| if (work_dir != NULL && strlen(work_dir) > 0 && strchr(work_dir, ' ') == NULL) { | ||
| execute_cmd("/bin/rmdir %s/* ", work_dir); | ||
| execute_cmd("/bin/rmdir %s/log/* ", work_dir); | ||
| } |
There was a problem hiding this comment.
The new work_dir guard only wraps the rmdir calls, but umount() already uses work_dir earlier in chdir(work_dir), strlen(work_dir), and multiple execute_cmd("...%s...", work_dir) calls. If work_dir can be NULL/unsafe, this check is ineffective (and chdir/strlen may crash); if the goal is to reduce shell injection risk, validating/escaping work_dir (or avoiding system() entirely) needs to happen before any use of it.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| if (strlen(userfile) > 0 && strchr(userfile, ' ') == NULL) { | ||
| execute_cmd("rm %s",userfile); | ||
| } |
There was a problem hiding this comment.
execute_cmd("rm %s", userfile) builds a shell command via system() without quoting/escaping userfile. Since userfile includes basename(noip_file_name) read from output.name, a crafted value containing shell metacharacters (e.g. ;, $(...), backticks) could lead to command injection. Prefer deleting via unlink()/remove() (no shell), or at minimum ensure robust escaping/quoting (spaces-only checks are not sufficient).
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…#1139) * Initial plan * Move work_dir validation to start of umount() before any use Co-authored-by: zhblue <[email protected]> Agent-Logs-Url: https://github.com/zhblue/hustoj/sessions/1074fa3b-637f-456a-ad51-6cc660b292c9 --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: zhblue <[email protected]>
No description provided.