Potential fix for code scanning alert no. 5: Incorrect return-value check for a 'scanf'-like function#1119
Potential fix for code scanning alert no. 5: Incorrect return-value check for a 'scanf'-like function#1119
Conversation
…heck for a 'scanf'-like function Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a code scanning alert by improving the return value checking for fscanf calls. Instead of simply testing for a non-zero return value, the code now explicitly checks whether fscanf successfully read exactly one value (== 1), which is the correct approach for these single-format-specifier calls.
Changes:
- Updated two
fscanfreturn value checks to use== 1instead of implicit boolean conversion - Added consistent spacing around operators for better readability
Comments suppressed due to low confidence (1)
trunk/core/judge_client/judge_client.cc:2122
- The file pointer
fpnameis not checked for NULL before being used infscanf. Iffopenfails,fpnamewill be NULL, and passing NULL tofscanfwill cause undefined behavior. Add a NULL check immediately after thefopencall, similar to the pattern used at line 2133.
FILE * fpname=fopen(noip_file_name,"r");
if (fscanf(fpname, "%s", noip_file_name) == 1){
execute_cmd("/bin/cp '%s' %s/%s", infile, work_dir,basename(noip_file_name)); // 如果存在input.name则复制测试数据
execute_cmd("/usr/bin/chown judge %s/%s", work_dir,basename(noip_file_name)); // 修改属主
if(DEBUG) printf("NOIP filename:%s\n",noip_file_name);
}
fclose(fpname);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(DEBUG) printf("NOIP filename:%s\n",noip_file_name); | ||
| FILE * fpname=fopen(noip_file_name,"r"); | ||
| if(fscanf(fpname,"%s",noip_file_name)){ | ||
| if (fscanf(fpname, "%s", noip_file_name) == 1){ |
There was a problem hiding this comment.
The fscanf call uses %s without a field width specifier, which can lead to buffer overflow if the file contains a string longer than BUFFER_SIZE. Use a field width specifier like %4095s (assuming BUFFER_SIZE is 4096) to limit the number of characters read and prevent potential buffer overflow vulnerabilities.
| FILE * fpname = fopen(noip_file_name, "r"); | ||
| if (fpname != NULL){ | ||
| if (fscanf(fpname, "%s", noip_file_name) == 1) { | ||
| if (fscanf(fpname, "%s", noip_file_name) == 1){ |
There was a problem hiding this comment.
The fscanf call uses %s without a field width specifier, which can lead to buffer overflow if the file contains a string longer than BUFFER_SIZE. Use a field width specifier like %4095s (assuming BUFFER_SIZE is 4096) to limit the number of characters read and prevent potential buffer overflow vulnerabilities.
* Refactor contest ranking logic in contestrank.xls.php * Sanitize nicknames with additional prefix check * Remove '../' from file names in problem import Sanitize file names by removing '../' to prevent directory traversal vulnerabilities. * Add exit to restrict access to proxy.php Added exit statement to prevent unauthorized access. * Update problem_import_hoj.php * Update problem_import_hydro.php * Update problem_import_md.php * Update problem_import_qduoj.php * Update problem_import_syzoj.php * Update problem_import_tyvj.php * Update problem_import_unkownoj.php * Update problem_import_hoj.php * Update my_func.inc.php * Update problem_import.php * Update problem_import_qduoj.php * Update problem_import_hoj.php * Update problem_import_qduoj.php * Update difficulty control standards in common.php * Update mail.php * Fix HTML form attributes in mail.php * Update mail.php * Potential fix for code scanning alert no. 4: Time-of-check time-of-use filesystem race condition (#1118) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for code scanning alert no. 6: Incorrect return-value check for a 'scanf'-like function (#1120) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for code scanning alert no. 5: Incorrect return-value check for a 'scanf'-like function (#1119) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Update date formatting in submit.php Replaced strftime with date for better date formatting. * Update submit.php --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Potential fix for https://github.com/zhblue/hustoj/security/code-scanning/5
In general,
scanf-family calls should compare their return value to the exact number of expected conversions, or check for< expected/== EOF, instead of simply testing for non‑zero. Here, eachfscanfcall has a single%sformat, so the expected successful assignment count is 1. The best minimal fix is to explicitly testfscanf(...) == 1before usingnoip_file_name, and to treat any other result (includingEOF) as failure by skipping the body.Concretely:
prepare_filesat line 2117, changeif(fscanf(fpname,"%s",noip_file_name)){toif (fscanf(fpname, "%s", noip_file_name) == 1) {.if(fscanf(fpname,"%s",noip_file_name)){toif (fscanf(fpname, "%s", noip_file_name) == 1) {.No new headers or helper methods are required; we only adjust the conditionals around existing
fscanfcalls. This preserves existing behavior for the successful-read case while correctly handling both 0 andEOFas failure.Suggested fixes powered by Copilot Autofix. Review carefully before merging.