#1168 closed Defect (fixed)
Very stupid bug on FILE_INFO::verify_file_certs()
Reported by: | matszpk | Owned by: | |
---|---|---|---|
Priority: | Critical | Milestone: | Undetermined |
Component: | Undetermined | Version: | 6.12.34 |
Keywords: | Cc: |
Description
while (dir_scan(file, dir, sizeof(file))) {
should be replaced by:
while (!dir_scan(file, dir, sizeof(file))) {
dir_scan returns zero if successfully reads next dir entry.
Change History (3)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 Changed 12 years ago by
Note: See
TracTickets for help on using
tickets.
I can confirm the issue to be still present with 7.0.34 and to effect also lib/crypt.cpp. The patch below (compiles here) presumably fixes both. When e.g. applied on empty directories, like while changing projects, possibly, I could imagine the code to have side effects that might contribute to https://boinc.berkeley.edu/trac/ticket/1203 . Just a hunch.
Index: boinc/client/cs_files.cpp
===================================================================
--- boinc.orig/client/cs_files.cpp 2012-08-30 21:03:35.490956131 +0200
+++ boinc/client/cs_files.cpp 2012-09-02 10:40:36.478032727 +0200
@@ -90,12 +90,12 @@
Is app signed by one of the Application Certifiers?
bool FILE_INFO::verify_file_certs() {
- char file[256];
+ char file[MAXPATHLEN];
bool retval = false;
if (!is_dir(CERTIFICATE_DIRECTORY)) return false;
DIRREF dir = dir_open(CERTIFICATE_DIRECTORY);
- while (dir_scan(file, dir, sizeof(file))) {
+ while (!dir_scan(file, dir, sizeof(file))) {
if (cert_verify_file(cert_sigs, file, CERTIFICATE_DIRECTORY)) {
msg_printf(project, MSG_INFO,
"Signature verified using certificate %s", file
@@ -156,7 +156,7 @@
int FILE_INFO::verify_file(
bool verify_contents, bool show_errors, bool allow_async
) {
- char cksum[64], pathname[256];
+ char cksum[64], pathname[MAXPATHLEN];
bool verified;
int retval;
double size, local_nbytes;
@@ -177,7 +177,9 @@
if (download_gzipped && !boinc_file_exists(pathname)) {
char gzpath[MAXPATHLEN];
- sprintf(gzpath, "%s.gz", pathname);
+ snprintf(gzpath, sizeof(gzpath), "%s.gz", pathname);
+ FIXME: a distinction is missing for the case that the .gz suffix goes beyond the MAXPATHLEN and is hence not found because
+ one should then not reperform the download as intended below
if (boinc_file_exists(gzpath) ) {
if (allow_async && nbytes > ASYNC_FILE_THRESHOLD) {
ASYNC_VERIFY* avp = new ASYNC_VERIFY;
Index: boinc/lib/crypt.cpp
===================================================================
--- boinc.orig/lib/crypt.cpp 2012-09-02 10:27:39.420339596 +0200
+++ boinc/lib/crypt.cpp 2012-09-02 10:42:50.568254414 +0200
@@ -604,7 +604,7 @@
DIRREF dir = dir_open(certPath);
char file[MAXPATHLEN];
- while (dir_scan(file, dir, sizeof(file))) {
+ while (!dir_scan(file, dir, sizeof(file))) {
char fpath[MAXPATHLEN];
snprintf(fpath, sizeof(fpath), "%s/%s", certPath, file);
TODO : replace '128'