diff --git a/doc/TODO b/doc/TODO index a0b4d5285d..42a93b9383 100644 --- a/doc/TODO +++ b/doc/TODO @@ -82,7 +82,7 @@ R - learn from ben about his openssl-reinitialization-trick to forbidden ports when fascistfirewall blocks all good dirservers. 0.0.9 and beyond: - - fix sprintf's to snprintf's? + o fix sprintf's to snprintf's? . Make intro points and rendezvous points accept $KEYID in addition to nicknames. o Specify diff --git a/src/common/log.c b/src/common/log.c index 3a64750ca0..06da75839d 100644 --- a/src/common/log.c +++ b/src/common/log.c @@ -142,7 +142,7 @@ static INLINE char *format_msg(char *buf, size_t buf_len, /** Helper: sends a message to the appropriate logfiles, at loglevel * severity. If provided, funcname is prepended to the - * message. The actual message is derived as from vsprintf(format,ap). + * message. The actual message is derived as from vsnprintf(format,ap). */ static void logv(int severity, const char *funcname, const char *format, va_list ap) diff --git a/src/common/tortls.c b/src/common/tortls.c index 17865d7294..84366176e6 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -294,12 +294,12 @@ tor_tls_context_new(crypto_pk_env_t *identity, EVP_PKEY *pkey = NULL; tor_tls_context *result = NULL; X509 *cert = NULL, *idcert = NULL; - char nn2[1024]; + char nn2[128]; int client_only; SSL_CTX **ctx; if (!nickname) nickname = "null"; - sprintf(nn2, "%s ", nickname); + snprintf(nn2, sizeof(nn2), "%s ", nickname); tor_tls_init(); diff --git a/src/common/util.c b/src/common/util.c index 023463bcf3..f94088ec9a 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1763,6 +1763,7 @@ char *expand_filename(const char *filename) tor_assert(filename); /* XXXX Should eventually check for ~username/ */ if (!strncmp(filename,"~/",2)) { + size_t len; const char *home = getenv("HOME"); char *result; if (!home) { @@ -1770,8 +1771,9 @@ char *expand_filename(const char *filename) return NULL; } /* minus two characters for ~/, plus one for /, plus one for NUL. */ - result = tor_malloc(strlen(home)+strlen(filename)+16); - sprintf(result,"%s/%s",home,filename+2); + len = strlen(home)+strlen(filename)+16; + result = tor_malloc(len); + snprintf(result,len,"%s/%s",home,filename+2); return result; } else { return tor_strdup(filename); diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 5b5c8e9a60..f9a369e59c 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1105,7 +1105,7 @@ static routerinfo_t *choose_good_entry_server(cpath_build_state_t *state) for(i=0; i < smartlist_len(rl->routers); i++) { r = smartlist_get(rl->routers, i); - sprintf(buf, "%d", r->or_port); + snprintf(buf, sizeof(buf), "%d", r->or_port); if(!smartlist_string_isin(options.FirewallPorts, buf)) smartlist_add(excluded, r); } diff --git a/src/or/directory.c b/src/or/directory.c index 8325988704..45f69a3c3d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -131,7 +131,7 @@ directory_post_to_dirservers(uint8_t purpose, const char *payload, * descriptor -- those use Tor. */ if (options.FascistFirewall && purpose == DIR_PURPOSE_UPLOAD_DIR && !options.HttpProxy) { - sprintf(buf,"%d",ds->dir_port); + snprintf(buf,sizeof(buf),"%d",ds->dir_port); if (!smartlist_string_isin(options.FirewallPorts, buf)) continue; } @@ -325,10 +325,10 @@ directory_send_command(connection_t *conn, const char *platform, if(conn->port == 80) { strlcpy(hoststring, conn->address, sizeof(hoststring)); } else { - sprintf(hoststring, "%s:%d", conn->address, conn->port); + snprintf(hoststring, sizeof(hoststring),"%s:%d",conn->address, conn->port); } if(options.HttpProxy) { - sprintf(proxystring, "http://%s", hoststring); + snprintf(proxystring, sizeof(proxystring),"http://%s", hoststring); } else { proxystring[0] = 0; } @@ -361,7 +361,7 @@ directory_send_command(connection_t *conn, const char *platform, conn->rend_query[payload_len] = 0; httpcommand = "GET"; - sprintf(url, "%s/rendezvous/%s", use_newer ? "/tor" : "", payload); + snprintf(url, sizeof(url), "%s/rendezvous/%s", use_newer ? "/tor" : "", payload); /* XXX We're using payload here to mean something other than * payload of the http post. This is probably bad, and should @@ -373,7 +373,7 @@ directory_send_command(connection_t *conn, const char *platform, case DIR_PURPOSE_UPLOAD_RENDDESC: tor_assert(payload); httpcommand = "POST"; - sprintf(url, "%s/rendezvous/publish", use_newer ? "/tor" : ""); + snprintf(url, sizeof(url), "%s/rendezvous/publish", use_newer ? "/tor" : ""); break; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 74409fe528..d78ab7c45c 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -835,7 +835,7 @@ static int generate_runningrouters(crypto_pk_env_t *private_key) #endif published_on = time(NULL); format_iso_time(published, published_on); - sprintf(s, "network-status\n" + snprintf(s, len, "network-status\n" "published %s\n" "router-status %s\n" "opt dir-signing-key %s\n" diff --git a/src/or/main.c b/src/or/main.c index 76aff1ac02..3adb12062f 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -757,7 +757,7 @@ static int do_hup(void) { } if(authdir_mode()) { /* reload the approved-routers file */ - sprintf(keydir,"%s/approved-routers", get_data_directory(&options)); + snprintf(keydir,sizeof(keydir),"%s/approved-routers", get_data_directory(&options)); log_fn(LOG_INFO,"Reloading approved fingerprints from %s...",keydir); if(dirserv_parse_fingerprint_file(keydir) < 0) { log_fn(LOG_WARN, "Error reloading fingerprints. Continuing with old list."); @@ -772,7 +772,7 @@ static int do_hup(void) { dnsworkers_rotate(); /* Rebuild fresh descriptor as needed. */ router_rebuild_descriptor(); - sprintf(keydir,"%s/router.desc", get_data_directory(&options)); + snprintf(keydir,sizeof(keydir),"%s/router.desc", get_data_directory(&options)); log_fn(LOG_INFO,"Dumping descriptor to %s...",keydir); if (write_str_to_file(keydir, router_get_my_descriptor(), 0)) { return -1; diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ddf54d836c..34711c8e54 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -309,7 +309,7 @@ int rend_service_load_keys(void) log_fn(LOG_WARN, "Directory name too long: '%s'", s->directory); return -1; } - sprintf(buf, "%s.onion\n", s->service_id); + snprintf(buf, sizeof(buf),"%s.onion\n", s->service_id); if (write_str_to_file(fname,buf,0)<0) return -1; } diff --git a/src/or/rephist.c b/src/or/rephist.c index 6838ceaf10..b87217280d 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -528,7 +528,7 @@ char *rep_hist_get_bandwidth_lines(void) for (r=0;r<2;++r) { b = r?read_array:write_array; format_iso_time(t, b->next_period-NUM_SECS_BW_SUM_INTERVAL); - sprintf(cp, "opt %s %s (%d s) ", r?"read-history ":"write-history", t, + snprintf(cp, len-(cp-buf), "opt %s %s (%d s) ", r?"read-history ":"write-history", t, NUM_SECS_BW_SUM_INTERVAL); cp += strlen(cp); @@ -543,9 +543,9 @@ char *rep_hist_get_bandwidth_lines(void) for (n=0; nnum_maxes_set; ++n,++i) { while (i >= NUM_TOTALS) i -= NUM_TOTALS; if (n==(b->num_maxes_set-1)) - sprintf(cp, "%d", b->totals[i]); + snprintf(cp, len-(cp-buf), "%d", b->totals[i]); else - sprintf(cp, "%d,", b->totals[i]); + snprintf(cp, len-(cp-buf), "%d,", b->totals[i]); cp += strlen(cp); } strcat(cp, "\n"); diff --git a/src/or/router.c b/src/or/router.c index 3475882cc8..e96b6b7752 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -101,9 +101,10 @@ void rotate_onion_key(void) char fname[512]; char fname_prev[512]; crypto_pk_env_t *prkey; - sprintf(fname,"%s/keys/secret_onion_key",get_data_directory(&options)); - sprintf(fname_prev,"%s/keys/secret_onion_key.old", - get_data_directory(&options)); + snprintf(fname,sizeof(fname), + "%s/keys/secret_onion_key",get_data_directory(&options)); + snprintf(fname_prev,sizeof(fname_prev), + "%s/keys/secret_onion_key.old",get_data_directory(&options)); if (!(prkey = crypto_new_pk_env())) { log(LOG_ERR, "Error creating crypto environment."); goto error; @@ -263,27 +264,27 @@ int init_keys(void) { return -1; } /* Check the key directory. */ - sprintf(keydir,"%s/keys", datadir); + snprintf(keydir,sizeof(keydir),"%s/keys", datadir); if (check_private_dir(keydir, 1)) { return -1; } cp = keydir + strlen(keydir); /* End of string. */ /* 1. Read identity key. Make it if none is found. */ - sprintf(keydir,"%s/keys/identity.key",datadir); - sprintf(keydir2,"%s/keys/secret_id_key",datadir); + snprintf(keydir,sizeof(keydir),"%s/keys/identity.key",datadir); + snprintf(keydir2,sizeof(keydir2),"%s/keys/secret_id_key",datadir); log_fn(LOG_INFO,"Reading/making identity key %s...",keydir2); prkey = init_key_from_file_name_changed(keydir,keydir2); if (!prkey) return -1; set_identity_key(prkey); /* 2. Read onion key. Make it if none is found. */ - sprintf(keydir,"%s/keys/onion.key",datadir); - sprintf(keydir2,"%s/keys/secret_onion_key",datadir); + snprintf(keydir,sizeof(keydir),"%s/keys/onion.key",datadir); + snprintf(keydir2,sizeof(keydir2),"%s/keys/secret_onion_key",datadir); log_fn(LOG_INFO,"Reading/making onion key %s...",keydir2); prkey = init_key_from_file_name_changed(keydir,keydir2); if (!prkey) return -1; set_onion_key(prkey); - sprintf(keydir,"%s/keys/secret_onion_key.old",datadir); + snprintf(keydir,sizeof(keydir),"%s/keys/secret_onion_key.old",datadir); if (file_status(keydir) == FN_FILE) { prkey = init_key_from_file(keydir); if (prkey) @@ -315,13 +316,13 @@ int init_keys(void) { } } - sprintf(keydir,"%s/router.desc", datadir); + snprintf(keydir,sizeof(keydir),"%s/router.desc", datadir); log_fn(LOG_INFO,"Dumping descriptor to %s...",keydir); if (write_str_to_file(keydir, mydesc,0)) { return -1; } /* 5. Dump fingerprint to 'fingerprint' */ - sprintf(keydir,"%s/fingerprint", datadir); + snprintf(keydir,sizeof(keydir),"%s/fingerprint", datadir); log_fn(LOG_INFO,"Dumping fingerprint to %s...",keydir); tor_assert(strlen(options.Nickname) <= MAX_NICKNAME_LEN); strcpy(fingerprint, options.Nickname); @@ -337,7 +338,7 @@ int init_keys(void) { if(!authdir_mode()) return 0; /* 6. [authdirserver only] load approved-routers file */ - sprintf(keydir,"%s/approved-routers", datadir); + snprintf(keydir,sizeof(keydir),"%s/approved-routers", datadir); log_fn(LOG_INFO,"Loading approved fingerprints from %s...",keydir); if(dirserv_parse_fingerprint_file(keydir) < 0) { log_fn(LOG_ERR, "Error loading fingerprints"); @@ -349,7 +350,7 @@ int init_keys(void) { add_trusted_dir_server(options.Address, (uint16_t)options.DirPort, digest); } /* 7. [authdirserver only] load old directory, if it's there */ - sprintf(keydir,"%s/cached-directory", datadir); + snprintf(keydir,sizeof(keydir),"%s/cached-directory", datadir); log_fn(LOG_INFO,"Loading cached directory from %s...",keydir); cp = read_file_to_str(keydir,0); if(!cp) { diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 598a3412a2..9fa1453bb0 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -167,7 +167,7 @@ router_pick_directory_server_impl(int requireothers, int fascistfirewall) if(requireothers && router_is_me(router)) continue; if(fascistfirewall) { - sprintf(buf,"%d",router->dir_port); + snprintf(buf,sizeof(buf),"%d",router->dir_port); if (!smartlist_string_isin(options.FirewallPorts, buf)) continue; } @@ -202,7 +202,7 @@ router_pick_trusteddirserver_impl(int requireother, int fascistfirewall) !memcmp(me->identity_digest, d->digest, DIGEST_LEN)) continue; if (fascistfirewall) { - sprintf(buf,"%d",d->dir_port); + snprintf(buf,sizeof(buf),"%d",d->dir_port); if (!smartlist_string_isin(options.FirewallPorts, buf)) continue; } diff --git a/src/or/test.c b/src/or/test.c index 5eec99ed9e..fecf6be2bd 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -50,7 +50,7 @@ setup_directory() int r; if (is_setup) return; - sprintf(temp_dir, "/tmp/tor_test_%d", (int) getpid()); + snprintf(temp_dir, sizeof(temp_dir), "/tmp/tor_test_%d", (int) getpid()); #ifdef MS_WINDOWS r = mkdir(temp_dir); #else @@ -69,7 +69,7 @@ get_fname(const char *name) { static char buf[1024]; setup_directory(); - sprintf(buf,"%s/%s",temp_dir,name); + snprintf(buf,sizeof(buf),"%s/%s",temp_dir,name); return buf; }