summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/coverity_maint9
-rw-r--r--src/common/util.c33
-rw-r--r--src/common/util.h3
-rw-r--r--src/or/circuitlist.c4
-rw-r--r--src/or/config.c6
-rw-r--r--src/or/connection.c2
-rw-r--r--src/or/geoip.c6
-rw-r--r--src/or/policies.c2
-rw-r--r--src/or/rendservice.c2
-rw-r--r--src/or/rephist.c13
-rw-r--r--src/or/router.c4
-rw-r--r--src/or/routerparse.c6
-rw-r--r--src/test/test_addr.c3
13 files changed, 64 insertions, 29 deletions
diff --git a/changes/coverity_maint b/changes/coverity_maint
new file mode 100644
index 0000000000..e7be90a485
--- /dev/null
+++ b/changes/coverity_maint
@@ -0,0 +1,9 @@
+ o Code simplifications and refactoring:
+ - Remove some dead code as indicated by coverity.
+ - Remove a few dead assignments during router parsing. Found by coverity.
+ o Minor bugfixes:
+ - Add some forgotten return value checks during unit tests. Found
+ by coverity.
+ - Don't use 1-bit wide signed bit fields. Found by coverity.
+ - Fix a rare memory leak during stats writing. Found by coverity.
+
diff --git a/src/common/util.c b/src/common/util.c
index a5a6ea3e8b..629c33977b 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1678,15 +1678,20 @@ file_status(const char *fname)
* is group-readable, but in all cases we create the directory mode 0700.
* If CPD_CHECK_MODE_ONLY is set, then we don't alter the directory permissions
* if they are too permissive: we just return -1.
+ * When effective_user is not NULL, check permissions against the given user and
+ * its primary group.
*/
int
-check_private_dir(const char *dirname, cpd_check_t check)
+check_private_dir(const char *dirname, cpd_check_t check, const char *effective_user)
{
int r;
struct stat st;
char *f;
#ifndef MS_WINDOWS
int mask;
+ struct passwd *pw = NULL;
+ uid_t running_uid;
+ gid_t running_gid;
#endif
tor_assert(dirname);
@@ -1725,33 +1730,47 @@ check_private_dir(const char *dirname, cpd_check_t check)
return -1;
}
#ifndef MS_WINDOWS
- if (st.st_uid != getuid()) {
+ if (effective_user) {
+ /* Lookup the user and group information, if we have a problem, bail out. */
+ pw = getpwnam(effective_user);
+ if (pw == NULL) {
+ log_warn(LD_CONFIG, "Error setting configured user: %s not found", effective_user);
+ return -1;
+ }
+ running_uid = pw->pw_uid;
+ running_gid = pw->pw_gid;
+ } else {
+ running_uid = getuid();
+ running_gid = getgid();
+ }
+
+ if (st.st_uid != running_uid) {
struct passwd *pw = NULL;
char *process_ownername = NULL;
- pw = getpwuid(getuid());
+ pw = getpwuid(running_uid);
process_ownername = pw ? tor_strdup(pw->pw_name) : tor_strdup("<unknown>");
pw = getpwuid(st.st_uid);
log_warn(LD_FS, "%s is not owned by this user (%s, %d) but by "
"%s (%d). Perhaps you are running Tor as the wrong user?",
- dirname, process_ownername, (int)getuid(),
+ dirname, process_ownername, (int)running_uid,
pw ? pw->pw_name : "<unknown>", (int)st.st_uid);
tor_free(process_ownername);
return -1;
}
- if ((check & CPD_GROUP_OK) && st.st_gid != getgid()) {
+ if ((check & CPD_GROUP_OK) && st.st_gid != running_gid) {
struct group *gr;
char *process_groupname = NULL;
- gr = getgrgid(getgid());
+ gr = getgrgid(running_gid);
process_groupname = gr ? tor_strdup(gr->gr_name) : tor_strdup("<unknown>");
gr = getgrgid(st.st_gid);
log_warn(LD_FS, "%s is not owned by this group (%s, %d) but by group "
"%s (%d). Are you running Tor as the wrong user?",
- dirname, process_groupname, (int)getgid(),
+ dirname, process_groupname, (int)running_gid,
gr ? gr->gr_name : "<unknown>", (int)st.st_gid);
tor_free(process_groupname);
diff --git a/src/common/util.h b/src/common/util.h
index 2974ab7538..6496c42db8 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -292,7 +292,8 @@ typedef unsigned int cpd_check_t;
#define CPD_CHECK 2
#define CPD_GROUP_OK 4
#define CPD_CHECK_MODE_ONLY 8
-int check_private_dir(const char *dirname, cpd_check_t check);
+int check_private_dir(const char *dirname, cpd_check_t check,
+ const char *effective_user);
#define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
#define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
typedef struct open_file_t open_file_t;
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index f1f0cb2a63..91fb13faf7 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -773,8 +773,8 @@ circuit_get_by_circid_orconn_impl(circid_t circ_id, or_connection_t *conn)
return found->circuit;
return NULL;
-
/* The rest of this checks for bugs. Disabled by default. */
+ /* We comment it out because coverity complains otherwise.
{
circuit_t *circ;
for (circ=global_circuitlist;circ;circ = circ->next) {
@@ -793,7 +793,7 @@ circuit_get_by_circid_orconn_impl(circid_t circ_id, or_connection_t *conn)
}
}
return NULL;
- }
+ } */
}
/** Return a circ such that:
diff --git a/src/or/config.c b/src/or/config.c
index d663336796..53ecca274b 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1047,7 +1047,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
/* Ensure data directory is private; create if possible. */
if (check_private_dir(options->DataDirectory,
- running_tor ? CPD_CREATE : CPD_CHECK)<0) {
+ running_tor ? CPD_CREATE : CPD_CHECK,
+ options->User)<0) {
tor_asprintf(msg,
"Couldn't access/create private data directory \"%s\"",
options->DataDirectory);
@@ -1060,7 +1061,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
char *fn = tor_malloc(len);
tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status",
options->DataDirectory);
- if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK) < 0) {
+ if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK,
+ options->User) < 0) {
tor_asprintf(msg,
"Couldn't access/create private data directory \"%s\"", fn);
tor_free(fn);
diff --git a/src/or/connection.c b/src/or/connection.c
index 81f0cdf812..e0b5b96286 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -910,7 +910,7 @@ check_location_for_unix_socket(or_options_t *options, const char *path)
if (options->ControlSocketsGroupWritable)
flags |= CPD_GROUP_OK;
- if (check_private_dir(p, flags) < 0) {
+ if (check_private_dir(p, flags, options->User) < 0) {
char *escpath, *escdir;
escpath = esc_for_log(path);
escdir = esc_for_log(p);
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 71ed3bedf4..d369b45aa3 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -990,7 +990,7 @@ geoip_dirreq_stats_write(time_t now)
geoip_remove_old_clients(start_of_dirreq_stats_interval);
statsdir = get_datadir_fname("stats");
- if (check_private_dir(statsdir, CPD_CREATE) < 0)
+ if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "dirreq-stats");
data_v2 = geoip_get_client_history(GEOIP_CLIENT_NETWORKSTATUS_V2);
@@ -1229,7 +1229,7 @@ geoip_bridge_stats_write(time_t now)
/* Write it to disk. */
statsdir = get_datadir_fname("stats");
- if (check_private_dir(statsdir, CPD_CREATE) < 0)
+ if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "bridge-stats");
@@ -1324,7 +1324,7 @@ geoip_entry_stats_write(time_t now)
geoip_remove_old_clients(start_of_entry_stats_interval);
statsdir = get_datadir_fname("stats");
- if (check_private_dir(statsdir, CPD_CREATE) < 0)
+ if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "entry-stats");
data = geoip_get_client_history(GEOIP_CLIENT_CONNECT);
diff --git a/src/or/policies.c b/src/or/policies.c
index 6738b484ae..73e5106896 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -46,7 +46,7 @@ typedef struct policy_summary_item_t {
uint16_t prt_max; /**< Highest port number to accept/reject. */
uint64_t reject_count; /**< Number of IP-Addresses that are rejected to
this port range. */
- int accepted:1; /** Has this port already been accepted */
+ unsigned int accepted:1; /** Has this port already been accepted */
} policy_summary_item_t;
/** Private networks. This list is used in two places, once to expand the
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 79abc57ee1..f4dcd8a2ac 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -570,7 +570,7 @@ rend_service_load_keys(void)
s->directory);
/* Check/create directory */
- if (check_private_dir(s->directory, CPD_CREATE) < 0)
+ if (check_private_dir(s->directory, CPD_CREATE, get_options()->User) < 0)
return -1;
/* Load key */
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 8f359c717c..3cc618547e 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -2297,7 +2297,7 @@ rep_hist_exit_stats_write(time_t now)
/* Try to write to disk. */
statsdir = get_datadir_fname("stats");
- if (check_private_dir(statsdir, CPD_CREATE) < 0) {
+ if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
log_warn(LD_HIST, "Unable to create stats/ directory!");
goto done;
}
@@ -2391,8 +2391,7 @@ rep_hist_buffer_stats_add_circ(circuit_t *circ, time_t end_of_interval)
stat = tor_malloc_zero(sizeof(circ_buffer_stats_t));
stat->processed_cells = orcirc->processed_cells;
/* 1000.0 for s -> ms; 2.0 because of app-ward and exit-ward queues */
- stat->mean_num_cells_in_queue = interval_length == 0 ? 0.0 :
- (double) orcirc->total_cell_waiting_time /
+ stat->mean_num_cells_in_queue = (double) orcirc->total_cell_waiting_time /
(double) interval_length / 1000.0 / 2.0;
stat->mean_time_cells_in_queue =
(double) orcirc->total_cell_waiting_time /
@@ -2442,8 +2441,8 @@ rep_hist_buffer_stats_write(time_t now)
int processed_cells[SHARES], circs_in_share[SHARES],
number_of_circuits, i;
double queued_cells[SHARES], time_in_queue[SHARES];
- smartlist_t *str_build = smartlist_create();
- char *str = NULL, *buf=NULL;
+ smartlist_t *str_build = NULL;
+ char *str = NULL, *buf = NULL;
circuit_t *circ;
if (!start_of_buffer_stats_interval)
@@ -2451,6 +2450,8 @@ rep_hist_buffer_stats_write(time_t now)
if (start_of_buffer_stats_interval + WRITE_STATS_INTERVAL > now)
goto done; /* Not ready to write */
+ str_build = smartlist_create();
+
/* add current circuits to stats */
for (circ = _circuit_get_global_list(); circ; circ = circ->next)
rep_hist_buffer_stats_add_circ(circ, now);
@@ -2486,7 +2487,7 @@ rep_hist_buffer_stats_write(time_t now)
smartlist_clear(circuits_for_buffer_stats);
/* write to file */
statsdir = get_datadir_fname("stats");
- if (check_private_dir(statsdir, CPD_CREATE) < 0)
+ if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
goto done;
filename = get_datadir_fname2("stats", "buffer-stats");
out = start_writing_to_stdio_file(filename, OPEN_FLAGS_APPEND,
diff --git a/src/or/router.c b/src/or/router.c
index 34ea8babd2..3dcd3ddc4c 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -535,12 +535,12 @@ init_keys(void)
return 0;
}
/* Make sure DataDirectory exists, and is private. */
- if (check_private_dir(options->DataDirectory, CPD_CREATE)) {
+ if (check_private_dir(options->DataDirectory, CPD_CREATE, options->User)) {
return -1;
}
/* Check the key directory. */
keydir = get_datadir_fname("keys");
- if (check_private_dir(keydir, CPD_CREATE)) {
+ if (check_private_dir(keydir, CPD_CREATE, options->User)) {
tor_free(keydir);
return -1;
}
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index b146e5e3c9..67b238e747 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1534,10 +1534,10 @@ router_parse_entry_from_string(const char *s, const char *end,
}
}
- if ((tok = find_opt_by_keyword(tokens, K_CACHES_EXTRA_INFO)))
+ if (find_opt_by_keyword(tokens, K_CACHES_EXTRA_INFO))
router->caches_extra_info = 1;
- if ((tok = find_opt_by_keyword(tokens, K_ALLOW_SINGLE_HOP_EXITS)))
+ if (find_opt_by_keyword(tokens, K_ALLOW_SINGLE_HOP_EXITS))
router->allow_single_hop_exits = 1;
if ((tok = find_opt_by_keyword(tokens, K_EXTRA_INFO_DIGEST))) {
@@ -1550,7 +1550,7 @@ router_parse_entry_from_string(const char *s, const char *end,
}
}
- if ((tok = find_opt_by_keyword(tokens, K_HIDDEN_SERVICE_DIR))) {
+ if (find_opt_by_keyword(tokens, K_HIDDEN_SERVICE_DIR)) {
router->wants_to_be_hs_dir = 1;
}
diff --git a/src/test/test_addr.c b/src/test/test_addr.c
index 7629d95732..ec0c508978 100644
--- a/src/test/test_addr.c
+++ b/src/test/test_addr.c
@@ -438,13 +438,16 @@ test_addr_ip6_helpers(void)
/* test tor_addr_parse_mask_ports */
test_addr_mask_ports_parse("[::f]/17:47-95", AF_INET6,
0, 0, 0, 0x0000000f, 17, 47, 95);
+ test_streq(p1, "::f");
//test_addr_parse("[::fefe:4.1.1.7/120]:999-1000");
//test_addr_parse_check("::fefe:401:107", 120, 999, 1000);
test_addr_mask_ports_parse("[::ffff:4.1.1.7]/120:443", AF_INET6,
0, 0, 0x0000ffff, 0x04010107, 120, 443, 443);
+ test_streq(p1, "::ffff:4.1.1.7");
test_addr_mask_ports_parse("[abcd:2::44a:0]:2-65000", AF_INET6,
0xabcd0002, 0, 0, 0x044a0000, 128, 2, 65000);
+ test_streq(p1, "abcd:2::44a:0");
r=tor_addr_parse_mask_ports("[fefef::]/112", &t1, NULL, NULL, NULL);
test_assert(r == -1);
r=tor_addr_parse_mask_ports("efef::/112", &t1, NULL, NULL, NULL);
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion