diff --git a/api/boinc_api.C b/api/boinc_api.C index e52efeed78..a5c533a5f2 100644 --- a/api/boinc_api.C +++ b/api/boinc_api.C @@ -139,7 +139,7 @@ int boinc_get_init_data(APP_INIT_DATA& app_init_data) { // resolve XML soft links // -int boinc_resolve_filename(char *virtual_name, char *physical_name) { +int boinc_resolve_filename(char *virtual_name, char *physical_name, int len) { FILE *fp; char buf[512]; @@ -155,7 +155,7 @@ int boinc_resolve_filename(char *virtual_name, char *physical_name) { // If it's the XML tag, return its value, // otherwise, return the original file name // - parse_str( buf, "", physical_name); + parse_str(buf, "", physical_name, len); return 0; } @@ -246,7 +246,7 @@ double boinc_cpu_time() { } #ifdef _WIN32 -void CALLBACK on_timer( HWND hwnd, UINT uMsg, UINT idEvent, DWORD dwTime ) { +void CALLBACK on_timer(HWND hwnd, UINT uMsg, UINT idEvent, DWORD dwTime) { #else void on_timer(int a) { #endif @@ -273,7 +273,7 @@ void on_timer(int a) { int set_timer(double period) { int retval=0; #ifdef _WIN32 - retval = SetTimer( NULL, 0, (int)(period*1000), on_timer ); + retval = SetTimer(NULL, 0, (int)(period*1000), on_timer); #endif #if HAVE_SIGNAL_H @@ -329,8 +329,8 @@ int parse_init_data_file(FILE* f, APP_INIT_DATA& ai) { } continue; } - else if (parse_str(buf, "", ai.user_name)) continue; - else if (parse_str(buf, "", ai.team_name)) continue; + else if (parse_str(buf, "", ai.user_name, sizeof(ai.user_name))) continue; + else if (parse_str(buf, "", ai.team_name, sizeof(ai.team_name))) continue; else if (parse_double(buf, "", ai.total_cobblestones)) continue; else if (parse_double(buf, "", ai.recent_avg_cobblestones)) continue; else if (parse_double(buf, "", ai.wu_cpu_time)) continue; @@ -397,14 +397,14 @@ int parse_fd_init_file(FILE* f) { char buf[256],filename[256]; int filedesc; while (fgets(buf, 256, f)) { - if (parse_str(buf, "", filename)) { + if (parse_str(buf, "", filename, sizeof(filename))) { if (fgets(buf, 256, f)) { if (parse_int(buf, "", filedesc)) { freopen(filename, "r", stdin); fprintf(stderr, "opened input file %s\n", filename); } } - } else if (parse_str(buf, "", filename)) { + } else if (parse_str(buf, "", filename, sizeof(filename))) { if (fgets(buf, 256, f)) { if (parse_int(buf, "", filedesc)) { freopen(filename, "w", stdout); diff --git a/api/boinc_api.h b/api/boinc_api.h index 3f32660ecf..ec905ecabe 100755 --- a/api/boinc_api.h +++ b/api/boinc_api.h @@ -65,7 +65,7 @@ struct APP_INIT_DATA { extern int boinc_init(); extern int boinc_get_init_data(APP_INIT_DATA&); extern int boinc_finish(int); -extern int boinc_resolve_filename(char*, char*); +extern int boinc_resolve_filename(char*, char*, int len); extern bool boinc_time_to_checkpoint(); extern int boinc_checkpoint_completed(); extern int boinc_fraction_done(); diff --git a/apps/concat.C b/apps/concat.C index c84bbe27e2..2bac690b5d 100644 --- a/apps/concat.C +++ b/apps/concat.C @@ -32,9 +32,9 @@ int run_slow = 0; #define CHECKPOINT_FILE "concat_state" -int do_checkpoint(MFILE& mf, int filenum, int nchars ) { +int do_checkpoint(MFILE& mf, int filenum, int nchars) { int retval; - char resolved_name[512],res_name2[512]; + char res_name2[512]; FILE* f = fopen("temp", "w"); if (!f) return 1; @@ -43,7 +43,7 @@ int do_checkpoint(MFILE& mf, int filenum, int nchars ) { fprintf(stderr, "APP: concat checkpointing\n"); - boinc_resolve_filename( CHECKPOINT_FILE, res_name2 ); + boinc_resolve_filename(CHECKPOINT_FILE, res_name2, sizeof(res_name2)); retval = mf.flush(); if (retval) return retval; @@ -57,7 +57,7 @@ void file_append(FILE* in, MFILE &out, int skip, int filenum) { char buf[1]; int n,nread,retval; - fseek( in, skip, SEEK_SET ); + fseek(in, skip, SEEK_SET); nread = skip; while (1) { @@ -66,11 +66,11 @@ void file_append(FILE* in, MFILE &out, int skip, int filenum) { out.write(buf, 1, n); nread += n; - if( boinc_time_to_checkpoint() ) { - fprintf( stderr, "Checkpoint.\n" ); - retval = do_checkpoint( out, filenum, nread ); - if( retval ) { - fprintf( stderr, "APP: concat checkpoint failed %d\n", retval ); + if (boinc_time_to_checkpoint()) { + fprintf(stderr, "Checkpoint.\n"); + retval = do_checkpoint(out, filenum, nread); + if (retval) { + fprintf(stderr, "APP: concat checkpoint failed %d\n", retval); exit(1); } boinc_checkpoint_completed(); @@ -115,26 +115,26 @@ int main(int argc, char** argv) { fprintf(stderr, "APP: concat: argv[%d] is %s\n", i, argv[i]); if (!strcmp(argv[i], "-run_slow")) run_slow = 1; } - boinc_resolve_filename( CHECKPOINT_FILE, file_name ); - state = fopen( file_name, "r" ); - if( state ) { - fscanf( state, "%d %d", &file_num, &nchars ); + boinc_resolve_filename(CHECKPOINT_FILE, file_name, sizeof(file_name)); + state = fopen(file_name, "r"); + if (state) { + fscanf(state, "%d %d", &file_num, &nchars); mode = "a"; } else { file_num = (run_slow ? 2 : 1); nchars = 0; mode = "w"; } - boinc_resolve_filename( argv[argc-1], file_name ); - fprintf( stderr, "res: %s\n", file_name ); + boinc_resolve_filename(argv[argc-1], file_name, sizeof(file_name)); + fprintf(stderr, "res: %s\n", file_name); retval = out.open(file_name, mode); if (retval) { fprintf(stderr, "APP: concat: can't open out file %s\n", argv[argc-1]); exit(1); } for (i=file_num; i %c\0", c, toupper(c) ); + sprintf(the_char, "%c -> %c\0", c, toupper(c)); c = toupper(c); out._putchar(c); nchars++; @@ -241,14 +241,14 @@ int DrawGLScene(GLvoid) // Here's Where We Do All The Drawing renderBitmapString(xPos,yPos,GLUT_BITMAP_HELVETICA_12,the_char); xPos += xDelta; yPos += yDelta; - if( xPos < -1 || xPos > 1 ) xDelta *= -1; - if( yPos < -1 || yPos > 1 ) yDelta *= -1; + if (xPos < -1 || xPos > 1) xDelta *= -1; + if (yPos < -1 || yPos > 1) yDelta *= -1; - sprintf( text, "User: %s", uc_aid.user_name ); + sprintf(text, "User: %s", uc_aid.user_name); renderBitmapString(-1.3,1.1,GLUT_BITMAP_HELVETICA_12, text); - sprintf( text, "Team: %s", uc_aid.team_name ); + sprintf(text, "Team: %s", uc_aid.team_name); renderBitmapString(-1.3,1.0,GLUT_BITMAP_HELVETICA_12, text); - sprintf( text, "CPU Time: %f", uc_aid.wu_cpu_time ); + sprintf(text, "CPU Time: %f", uc_aid.wu_cpu_time); renderBitmapString(-1.3,0.9,GLUT_BITMAP_HELVETICA_12, text); return TRUE; // Everything Went OK diff --git a/checkin_notes b/checkin_notes index 823744a95f..185d13d783 100755 --- a/checkin_notes +++ b/checkin_notes @@ -1858,3 +1858,27 @@ Eric September 18, 2002 graphics_api.C graphics_api.h windows_opengl.cpp + +David Sept 22, 2002 + - Various changes to prevent buffer overrun in servers; + parse_str and parse_attr now take a buffer length arg + + api/ + boinc_api.C,h + apps/ + concat.C + upper_case.C + client/ + app.C + client_types.C + file_xfer.C + hostinfo.C + scheduler_op.C + doc/ + account.html + lib/ + parse.C,h + sched/ + file_upload_handler.C + handle_request.C + server_types.C,h diff --git a/client/app.C b/client/app.C index 5b56e11a25..6497168019 100644 --- a/client/app.C +++ b/client/app.C @@ -295,7 +295,7 @@ int ACTIVE_TASK::start(bool first_time) { argv[0] = exec_name; parse_command_line(wup->command_line, argv+1); if (log_flags.task_debug) print_argv(argv); - boinc_resolve_filename(exec_name, temp); + boinc_resolve_filename(exec_name, temp, sizeof(temp)); retval = execv(temp, argv); fprintf(stderr, "execv failed: %d\n", retval); perror("execv"); @@ -664,14 +664,16 @@ bool ACTIVE_TASK_SET::poll_time() { } // Gets the next available free slot, or returns -1 if all slots are full +// TODO: don't use malloc here // int ACTIVE_TASK_SET::get_free_slot(int total_slots) { - int i; + unsigned int i; char *slot_status; - if (active_tasks.size() >= total_slots) + if (active_tasks.size() >= (unsigned int)total_slots) { return -1; - + } + slot_status = (char *)calloc( sizeof(char), total_slots ); if (!slot_status) return -1; @@ -681,7 +683,7 @@ int ACTIVE_TASK_SET::get_free_slot(int total_slots) { } } - for (i=0; i", result_name)) continue; - else if (parse_str(buf, "", project_master_url)) continue; + else if (parse_str(buf, "", result_name, sizeof(result_name))) continue; + else if (parse_str(buf, "", project_master_url, sizeof(project_master_url))) continue; else if (parse_int(buf, "", app_version_num)) continue; else if (parse_int(buf, "", slot)) continue; else if (parse_double(buf, "", checkpoint_cpu_time)) continue; diff --git a/client/client_types.C b/client/client_types.C index c798b3c4ca..a7782f794a 100644 --- a/client/client_types.C +++ b/client/client_types.C @@ -66,8 +66,8 @@ int PROJECT::parse_prefs(FILE* in) { strcpy(authenticator, ""); while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", master_url)) continue; - else if (parse_str(buf, "", authenticator)) continue; + else if (parse_str(buf, "", master_url, sizeof(master_url))) continue; + else if (parse_str(buf, "", authenticator, sizeof(authenticator))) continue; else if (parse_double(buf, "", resource_share)) continue; else if (match_tag(buf, "")) { retval = dup_element_contents(in, "", &p); @@ -95,13 +95,13 @@ int PROJECT::parse_state(FILE* in) { nrpc_failures = 0; while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", string.text)) { + else if (parse_str(buf, "", string.text, sizeof(string.text))) { scheduler_urls.push_back(string); continue; } - else if (parse_str(buf, "", master_url)) continue; - else if (parse_str(buf, "", project_name)) continue; - else if (parse_str(buf, "", user_name)) continue; + else if (parse_str(buf, "", master_url, sizeof(master_url))) continue; + else if (parse_str(buf, "", project_name, sizeof(project_name))) continue; + else if (parse_str(buf, "", user_name, sizeof(user_name))) continue; else if (parse_double(buf, "", total_credit)) continue; else if (parse_double(buf, "", expavg_credit)) continue; else if (parse_int(buf, "", rpc_seqno)) continue; @@ -202,7 +202,7 @@ int APP::parse(FILE* in) { project = NULL; while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", name)) continue; + else if (parse_str(buf, "", name, sizeof(name))) continue; else fprintf(stderr, "APP::parse(): unrecognized: %s\n", buf); } return ERR_XML_PARSE; @@ -287,8 +287,8 @@ int FILE_INFO::parse(FILE* in, bool from_server) { if (from_server) { strcatdup(signed_xml, buf); } - if (parse_str(buf, "", name)) continue; - else if (parse_str(buf, "", url.text)) { + if (parse_str(buf, "", name, sizeof(name))) continue; + else if (parse_str(buf, "", url.text, sizeof(url.text))) { urls.push_back(url); continue; } @@ -296,7 +296,7 @@ int FILE_INFO::parse(FILE* in, bool from_server) { dup_element_contents(in, "", &file_signature); continue; } - else if (parse_str(buf, "", md5_cksum)) continue; + else if (parse_str(buf, "", md5_cksum, sizeof(md5_cksum))) continue; else if (parse_double(buf, "", nbytes)) continue; else if (parse_double(buf, "", max_nbytes)) continue; else if (match_tag(buf, "")) generated_locally = true; @@ -409,7 +409,7 @@ int APP_VERSION::parse(FILE* in) { project = NULL; while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", app_name)) continue; + else if (parse_str(buf, "", app_name, sizeof(app_name))) continue; else if (match_tag(buf, "")) { file_ref.parse(in); app_files.push_back(file_ref); @@ -449,8 +449,8 @@ int FILE_REF::parse(FILE* in) { main_program = false; while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", file_name)) continue; - else if (parse_str(buf, "", open_name)) continue; + else if (parse_str(buf, "", file_name, sizeof(file_name))) continue; + else if (parse_str(buf, "", open_name, sizeof(open_name))) continue; else if (parse_int(buf, "", fd)) continue; else if (match_tag(buf, "")) main_program = true; else fprintf(stderr, "FILE_REF::parse(): unrecognized: %s\n", buf); @@ -492,11 +492,11 @@ int WORKUNIT::parse(FILE* in) { seconds_to_complete = 0; while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", name)) continue; - else if (parse_str(buf, "", app_name)) continue; + else if (parse_str(buf, "", name, sizeof(name))) continue; + else if (parse_str(buf, "", app_name, sizeof(app_name))) continue; else if (parse_int(buf, "", version_num)) continue; - else if (parse_str(buf, "", command_line)) continue; - else if (parse_str(buf, "", env_vars)) continue; + else if (parse_str(buf, "", command_line, sizeof(command_line))) continue; + else if (parse_str(buf, "", env_vars, sizeof(env_vars))) continue; else if (parse_double(buf, "", seconds_to_complete)) continue; else if (match_tag(buf, "")) { file_ref.parse(in); @@ -534,7 +534,7 @@ int RESULT::parse_ack(FILE* in) { strcpy(name, ""); while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", name)) continue; + else if (parse_str(buf, "", name, sizeof(name))) continue; else fprintf(stderr, "RESULT::parse(): unrecognized: %s\n", buf); } return 1; @@ -564,8 +564,8 @@ int RESULT::parse_server(FILE* in) { clear(); while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - if (parse_str(buf, "", name)) continue; - if (parse_str(buf, "", wu_name)) continue; + if (parse_str(buf, "", name, sizeof(name))) continue; + if (parse_str(buf, "", wu_name, sizeof(wu_name))) continue; if (parse_int(buf, "", report_deadline)) continue; if (match_tag(buf, "")) { file_ref.parse(in); @@ -586,8 +586,8 @@ int RESULT::parse_state(FILE* in) { clear(); while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; - if (parse_str(buf, "", name)) continue; - if (parse_str(buf, "", wu_name)) continue; + if (parse_str(buf, "", name, sizeof(name))) continue; + if (parse_str(buf, "", wu_name, sizeof(wu_name))) continue; if (parse_int(buf, "", report_deadline)) continue; if (match_tag(buf, "")) { file_ref.parse(in); diff --git a/client/file_xfer.C b/client/file_xfer.C index 13c56bcab9..b2040bbea8 100644 --- a/client/file_xfer.C +++ b/client/file_xfer.C @@ -120,7 +120,7 @@ int FILE_XFER::parse_server_response(double &offset) { parse_double(req1, "", offset); parse_int(req1, "", status); // TODO: decide what to do with error string - //if (!parse_str(req1, "", upload_offset) ) return -1; + //if (!parse_str(req1, "", upload_offset, sizeof(upload_offset)) ) return -1; return status; } diff --git a/client/hostinfo.C b/client/hostinfo.C index d8abae8edd..c7f1179136 100644 --- a/client/hostinfo.C +++ b/client/hostinfo.C @@ -33,17 +33,17 @@ int HOST_INFO::parse(FILE* in) { while (fgets(buf, 256, in)) { if (match_tag(buf, "")) return 0; else if (parse_int(buf, "", timezone)) continue; - else if (parse_str(buf, "", domain_name)) continue; - else if (parse_str(buf, "", ip_addr)) continue; + else if (parse_str(buf, "", domain_name, sizeof(domain_name))) continue; + else if (parse_str(buf, "", ip_addr, sizeof(ip_addr))) continue; else if (parse_int(buf, "", p_ncpus)) continue; - else if (parse_str(buf, "", p_vendor)) continue; - else if (parse_str(buf, "", p_model)) continue; + else if (parse_str(buf, "", p_vendor, sizeof(p_vendor))) continue; + else if (parse_str(buf, "", p_model, sizeof(p_model))) continue; else if (parse_double(buf, "", p_fpops)) continue; else if (parse_double(buf, "", p_iops)) continue; else if (parse_double(buf, "", p_membw)) continue; else if (parse_double(buf, "", p_calculated)) continue; - else if (parse_str(buf, "", os_name)) continue; - else if (parse_str(buf, "", os_version)) continue; + else if (parse_str(buf, "", os_name, sizeof(os_name))) continue; + else if (parse_str(buf, "", os_version, sizeof(os_version))) continue; else if (parse_double(buf, "", m_nbytes)) continue; else if (parse_double(buf, "", m_cache)) continue; else if (parse_double(buf, "", m_swap)) continue; diff --git a/client/scheduler_op.C b/client/scheduler_op.C index 97d15b1639..36c3095e62 100644 --- a/client/scheduler_op.C +++ b/client/scheduler_op.C @@ -163,7 +163,7 @@ int SCHEDULER_OP::parse_master_file(vector &urls) { } project->scheduler_urls.clear(); while (fgets(buf, 256, f)) { - if (parse_str(buf, "", str.text)) { + if (parse_str(buf, "", str.text, sizeof(str.text))) { urls.push_back(str); } } @@ -376,9 +376,9 @@ int SCHEDULER_REPLY::parse(FILE* in) { // Do nothing } else if (match_tag(buf, "")) { return 0; - } else if (parse_str(buf, "", project_name)) { + } else if (parse_str(buf, "", project_name, sizeof(project_name))) { continue; - } else if (parse_str(buf, "", user_name)) { + } else if (parse_str(buf, "", user_name, sizeof(user_name))) { continue; } else if (parse_double(buf, "", total_credit)) { continue; @@ -428,8 +428,8 @@ int SCHEDULER_REPLY::parse(FILE* in) { RESULT result; result.parse_ack(in); result_acks.push_back(result); - } else if (parse_str(buf, "
  • An email address.
  • A public "screen name" (real name or nickname). +This also serves as your login name for the project's web site.
  • A web password (used to log in to the project's web site).
  • Country (optional) @@ -30,6 +31,9 @@ That's it! You can go to the project web site to set your user preferences. +

    Security

    +

    +

    Multiple hosts under one account

    You can run BOINC on many hosts, all under one account. diff --git a/doc/server_security.html b/doc/server_security.html new file mode 100644 index 0000000000..1e66e71556 --- /dev/null +++ b/doc/server_security.html @@ -0,0 +1,56 @@ +

    Notes on server security

    +

    +BOINC scheduling servers, data servers, and web servers +must be accessible via HTTP (port 80) +and therefore are potential targets for network attacks. +The possibility exists that BOINC software could +have vulnerabilities to such attacks. + +

    Scheduling server

    +

    +All network input to the scheduling server is read by calls of the form +

    fgets(buf, 256, stdin);
    +where buf is a 256-byte buffer. +There is no possibility of a buffer overrun from these calls. +In some cases data is copied out of the buffer to a second buffer; +this is done using functions +(
    parse_str()
    , 
    parse_attr()
    and 
    strncpy()
    ) +that take a buffer-length argument, +so again there can be no buffer overruns. +

    +The scheduling server doesn't run any secondary programs. +

    +The scheduling server creates disk files in which it stores +request and reply messages. +These files have names of the form +PATH/sched_req_PID +where PATH is a compiled-in directory name (e.g. /tmp) +and PID is the server process ID. +There is no possibility of the server creating +executable files, or files in other directories. + +

    File upload handler

    +

    +The file upload handler parses its input +in the same way as the scheduling servers, +except for file data. +This data is read using fread() in fixed-sized increments. +So there are no buffer overruns. +

    +The file upload handler reads and writes +files with names of the form BOINC_UPLOAD_DIR/filename, +where BOINC_UPLOAD_DIR is a compiled constant +for the directory where data files are stored. +"filename" is checked for ".." and such requests are ignored. +Hence files outside the directory cannot be read or written. +

    +The only place where files are created (in copy_socket_to_file()) +is a call "fopen(path, "wb");". +Hence no executable files or links are created. + + +

    PHP files

    +

    +The PHP files in the participant and administrative +web directories (html_user and html_ops) +make no calls that access local files or run programs. diff --git a/lib/parse.C b/lib/parse.C index cfc0aa7948..9547237950 100644 --- a/lib/parse.C +++ b/lib/parse.C @@ -34,11 +34,17 @@ #include "parse.h" #include "error_numbers.h" +// return true if the tag appears in the line +// bool match_tag(char* buf, char* tag) { if (strstr(buf, tag)) return true; return false; } +// parse an integer of the form 1234 +// return true if it's there +// Note: this doesn't check for the end tag +// bool parse_int(char* buf, char* tag, int& x) { char* p = strstr(buf, tag); if (!p) return false; @@ -46,6 +52,8 @@ bool parse_int(char* buf, char* tag, int& x) { return true; } +// Same, for doubles +// bool parse_double(char* buf, char* tag, double& x) { char* p = strstr(buf, tag); if (!p) return false; @@ -53,20 +61,25 @@ bool parse_double(char* buf, char* tag, double& x) { return true; } -bool parse_str(char* buf, char* tag, char* x) { +// parse a string of the form string +// +bool parse_str(char* buf, char* tag, char* dest, int len) { char* p = strstr(buf, tag); if (!p) return false; p = strchr(p, '>'); char* q = strchr(p+1, '<'); *q = 0; - strcpy(x, p+1); + strncpy(dest, p+1, len); + dest[len-1] = 0; return true; } -void parse_attr(char* buf, char* name, char* out) { +// parse a string of the form name="string" +// +void parse_attr(char* buf, char* name, char* dest, int len) { char* p, *q; - strcpy(out, ""); + strcpy(dest, ""); p = strstr(buf, name); if (!p) return; p = strchr(p, '"'); @@ -74,7 +87,8 @@ void parse_attr(char* buf, char* name, char* out) { q = strchr(p+1, '"'); if (!q) return; *q = 0; - strcpy(out, p+1); + strncpy(dest, p+1, len); + dest[len-1] = 0; } void copy_stream(FILE* in, FILE* out) { @@ -87,6 +101,8 @@ void copy_stream(FILE* in, FILE* out) { } } +// append to a malloc'd string +// void strcatdup(char*& p, char* buf) { p = (char*)realloc(p, strlen(p) + strlen(buf)+1); if (!p) { @@ -96,6 +112,8 @@ void strcatdup(char*& p, char* buf) { strcat(p, buf); } +// copy from a file to a malloc'd string until the end tag is reached +// int dup_element_contents(FILE* in, char* end_tag, char** pp) { char buf[256]; @@ -111,6 +129,8 @@ int dup_element_contents(FILE* in, char* end_tag, char** pp) { return 1; } +// read a file into a malloc'd string +// int read_file_malloc(char* pathname, char*& str) { char buf[256]; FILE* f; diff --git a/lib/parse.h b/lib/parse.h index be13989e0a..57e672f974 100644 --- a/lib/parse.h +++ b/lib/parse.h @@ -22,8 +22,8 @@ extern bool parse(char*, char*); extern bool parse_int(char*, char*, int&); extern bool parse_double(char*, char*, double&); -extern bool parse_str(char*, char*, char*); -extern void parse_attr(char* buf, char* attrname, char* out); +extern bool parse_str(char*, char*, char*, int); +extern void parse_attr(char* buf, char* attrname, char* out, int len); extern bool match_tag(char*, char*); extern void copy_stream(FILE* in, FILE* out); extern void strcatdup(char*& p, char* buf); diff --git a/sched/file_upload_handler.C b/sched/file_upload_handler.C index 8367c80ce2..eb783498b7 100644 --- a/sched/file_upload_handler.C +++ b/sched/file_upload_handler.C @@ -64,7 +64,7 @@ int FILE_INFO::parse(FILE* in) { continue; } strcatdup(signed_xml, buf); - if (parse_str(buf, "", name)) continue; + if (parse_str(buf, "", name, sizeof(name))) continue; if (parse_double(buf, "", max_nbytes)) continue; //fprintf(stderr, "file_upload_handler (%s): FILE_INFO::parse: unrecognized: %s \n", BOINC_USER, buf); } @@ -158,7 +158,7 @@ int handle_request(FILE* in, R_RSA_PUBLIC_KEY& key) { continue; } // Handle a file size request - else if (parse_str(buf, "", file_name)) { + else if (parse_str(buf, "", file_name, sizeof(file_name))) { struct stat sbuf; // TODO: check to ensure path doesn't point somewhere bad // diff --git a/sched/handle_request.C b/sched/handle_request.C index 8d761ecad1..615f0dec7a 100644 --- a/sched/handle_request.C +++ b/sched/handle_request.C @@ -133,7 +133,10 @@ int authenticate_user(SCHEDULER_REQUEST& sreq, SCHEDULER_REPLY& reply) { reply.host.rpc_seqno = sreq.rpc_seqno; reply.host.rpc_time = time(0); } else { - strcpy(reply.user.authenticator, sreq.authenticator); + strncpy( + reply.user.authenticator, sreq.authenticator, + sizeof(reply.user.authenticator) + ); retval = db_user_lookup_auth(reply.user); if (retval) { strcpy(reply.message, "Invalid or missing authenticator"); @@ -170,10 +173,10 @@ int update_host_record(SCHEDULER_REQUEST& sreq, HOST& host) { int retval; host.timezone = sreq.host.timezone; - strcpy(host.domain_name, sreq.host.domain_name); - strcpy(host.serialnum, sreq.host.serialnum); + strncpy(host.domain_name, sreq.host.domain_name, sizeof(host.domain_name)); + strncpy(host.serialnum, sreq.host.serialnum, sizeof(host.serialnum)); if (strcmp(host.last_ip_addr, sreq.host.last_ip_addr)) { - strcpy(host.last_ip_addr, sreq.host.last_ip_addr); + strncpy(host.last_ip_addr, sreq.host.last_ip_addr, sizeof(host.last_ip_addr)); } else { host.nsame_ip_addr++; } @@ -181,14 +184,14 @@ int update_host_record(SCHEDULER_REQUEST& sreq, HOST& host) { host.connected_frac = sreq.host.connected_frac; host.active_frac = sreq.host.active_frac; host.p_ncpus = sreq.host.p_ncpus; - strcpy(host.p_vendor, sreq.host.p_vendor); // unlikely this will change - strcpy(host.p_model, sreq.host.p_model); + strncpy(host.p_vendor, sreq.host.p_vendor, sizeof(host.p_vendor)); // unlikely this will change + strncpy(host.p_model, sreq.host.p_model, sizeof(host.p_model)); host.p_fpops = sreq.host.p_fpops; host.p_iops = sreq.host.p_iops; host.p_membw = sreq.host.p_membw; host.p_calculated = sreq.host.p_calculated; - strcpy(host.os_name, sreq.host.os_name); - strcpy(host.os_version, sreq.host.os_version); + strncpy(host.os_name, sreq.host.os_name, sizeof(host.os_name)); + strncpy(host.os_version, sreq.host.os_version, sizeof(host.os_version)); host.m_nbytes = sreq.host.m_nbytes; host.m_cache = sreq.host.m_cache; host.m_swap = sreq.host.m_swap; @@ -209,7 +212,7 @@ int update_host_record(SCHEDULER_REQUEST& sreq, HOST& host) { // int handle_prefs(SCHEDULER_REQUEST& sreq, SCHEDULER_REPLY& reply) { if (sreq.prefs_mod_time > reply.user.prefs_mod_time && strlen(sreq.prefs_xml)) { - strcpy(reply.user.prefs, sreq.prefs_xml); + strncpy(reply.user.prefs, sreq.prefs_xml, sizeof(reply.user.prefs)); reply.user.prefs_mod_time = sreq.prefs_mod_time; if (reply.user.prefs_mod_time > (unsigned)time(0)) { reply.user.prefs_mod_time = (unsigned)time(0); @@ -232,7 +235,7 @@ int handle_results(SCHEDULER_REQUEST& sreq, SCHEDULER_REPLY& reply) { for (i=0; iname); + strncpy(result.name, rp->name, sizeof(result.name)); retval = db_result_lookup_name(result); if (retval) { printf("can't find result %s\n", rp->name); @@ -245,6 +248,14 @@ int handle_results(SCHEDULER_REQUEST& sreq, SCHEDULER_REPLY& reply) { continue; } + if (result.hostid != sreq.hostid) { + fprintf(stderr, + "got result from wrong host: %d %d\n", + result.hostid, sreq.hostid + ); + continue; + } + // TODO: handle error returns // result.hostid = reply.host.id; @@ -252,8 +263,8 @@ int handle_results(SCHEDULER_REQUEST& sreq, SCHEDULER_REPLY& reply) { result.exit_status = rp->exit_status; result.cpu_time = rp->cpu_time; result.state = RESULT_STATE_DONE; - strcpy(result.stderr_out, rp->stderr_out); - strcpy(result.xml_doc_out, rp->xml_doc_out); + strncpy(result.stderr_out, rp->stderr_out, sizeof(result.stderr_out)); + strncpy(result.xml_doc_out, rp->xml_doc_out, sizeof(result.xml_doc_out)); db_result_update(result); retval = db_workunit(result.workunitid, wu); @@ -346,7 +357,7 @@ int send_work( result.sent_time = time(0); sprintf(result.name, "result_%d", result.id); app = db.lookup_app(wu.appid); - strcpy(result.xml_doc_in, app->result_xml_template); + strncpy(result.xml_doc_in, app->result_xml_template, sizeof(result.xml_doc_in)); sprintf(prefix, "%s_", result.name); process_result_template( result.xml_doc_in, prefix, wu.name, result.name @@ -369,7 +380,7 @@ int send_work( return 0; } -// if the client has an old code sign key, +// if the client has an old code sign public key, // send it the new one, with a signature based on the old one. // If they don't have a code sign key, send them one // diff --git a/sched/server_types.C b/sched/server_types.C index b3aeecbb2f..8ac7ceebd3 100644 --- a/sched/server_types.C +++ b/sched/server_types.C @@ -49,10 +49,10 @@ int SCHEDULER_REQUEST::parse(FILE* fin) { if (!match_tag(buf, "")) return 1; while (fgets(buf, 256, fin)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", authenticator)) continue; + else if (parse_str(buf, "", authenticator, sizeof(authenticator))) continue; else if (parse_int(buf, "", hostid)) continue; else if (parse_int(buf, "", rpc_seqno)) continue; - else if (parse_str(buf, "", platform_name)) continue; + else if (parse_str(buf, "", platform_name, sizeof(platform_name))) continue; else if (parse_int(buf, "", core_client_version)) continue; else if (parse_int(buf, "", work_req_seconds)) continue; else if (parse_int(buf, "", (int)prefs_mod_time)) { @@ -98,6 +98,8 @@ SCHEDULER_REPLY::SCHEDULER_REPLY() { send_prefs = false; code_sign_key = 0; code_sign_key_signature = 0; + memset(&user, 0, sizeof(user)); + memset(&host, 0, sizeof(host)); } SCHEDULER_REPLY::~SCHEDULER_REPLY() { @@ -239,7 +241,7 @@ int RESULT::parse_from_client(FILE* fin) { memset(this, 0, sizeof(RESULT)); while (fgets(buf, 256, fin)) { if (match_tag(buf, "")) return 0; - else if (parse_str(buf, "", name)) continue; + else if (parse_str(buf, "", name, sizeof(name))) continue; else if (parse_int(buf, "", exit_status)) continue; else if (parse_double(buf, "", cpu_time)) continue; else if (match_tag(buf, "")) { @@ -273,18 +275,18 @@ int HOST::parse(FILE* fin) { while (fgets(buf, 256, fin)) { if (match_tag(buf, "")) return 0; else if (parse_int(buf, "", timezone)) continue; - else if (parse_str(buf, "", domain_name)) continue; - else if (parse_str(buf, "", serialnum)) continue; - else if (parse_str(buf, "", last_ip_addr)) continue; + else if (parse_str(buf, "", domain_name, sizeof(domain_name))) continue; + else if (parse_str(buf, "", serialnum, sizeof(serialnum))) continue; + else if (parse_str(buf, "", last_ip_addr, sizeof(last_ip_addr))) continue; else if (parse_int(buf, "", p_ncpus)) continue; - else if (parse_str(buf, "", p_vendor)) continue; - else if (parse_str(buf, "", p_model)) continue; + else if (parse_str(buf, "", p_vendor, sizeof(p_vendor))) continue; + else if (parse_str(buf, "", p_model, sizeof(p_model))) continue; else if (parse_double(buf, "", p_fpops)) continue; else if (parse_double(buf, "", p_iops)) continue; else if (parse_double(buf, "", p_membw)) continue; else if (parse_double(buf, "", p_calculated)) continue; - else if (parse_str(buf, "", os_name)) continue; - else if (parse_str(buf, "", os_version)) continue; + else if (parse_str(buf, "", os_name, sizeof(os_name))) continue; + else if (parse_str(buf, "", os_version, sizeof(os_version))) continue; else if (parse_double(buf, "", m_nbytes)) continue; else if (parse_double(buf, "", m_cache)) continue; else if (parse_double(buf, "", m_swap)) continue; @@ -322,62 +324,3 @@ int HOST::parse_net_stats(FILE* fin) { } return 1; } - -#if 0 -DB_CACHE::DB_CACHE() { -} - -int DB_CACHE::read_db() { - PLATFORM platform; - APP app; - APP_VERSION app_version; - - while (!db_platform_enum(platform)) { - platforms.push_back(platform); - } - while (!db_app_enum(app)) { - apps.push_back(app); - } - while (!db_app_version_enum(app_version)) { - app_versions.push_back(app_version); - } - return 0; -} - -PLATFORM* DB_CACHE::lookup_platform(char* name) { - unsigned int i; - assert(name!=NULL); - for (i=0; i=0); - for (i=0; iappid == appid && avp->platformid == platformid && avp->version_num == version) { - return avp; - } - } - return 0; -} -#endif diff --git a/sched/server_types.h b/sched/server_types.h index bee9a1b2ca..5ad403fdc8 100644 --- a/sched/server_types.h +++ b/sched/server_types.h @@ -44,6 +44,9 @@ struct SCHEDULER_REQUEST { int parse(FILE*); }; +// NOTE: if any field requires initialization, +// you must do it in the constructor. Nothing is zeroed by default. +// struct SCHEDULER_REPLY { int request_delay; // don't request again until this time elapses char message[1024];