Browse Source

Wipe all of the target space in tor_addr_{to,from}_sockaddr()

Otherwise we risk a subsequent memdup or memcpy copying
uninitialized RAM into some other place that might eventually expose
it.  Let's make sure that doesn't happen.

Closes ticket 14041
Nick Mathewson 9 years ago
parent
commit
d7ecdd645a
2 changed files with 10 additions and 2 deletions
  1. 5 0
      changes/bug14041
  2. 5 2
      src/common/address.c

+ 5 - 0
changes/bug14041

@@ -0,0 +1,5 @@
+  o Minor features (security):
+    - Clear all memory targetted by tor_addr_{to,from}_sockaddr(),
+      not just the part that's used. This makes it harder for data leak
+      bugs to occur in the event of other programming failures.
+      Resolves ticket 14041.

+ 5 - 2
src/common/address.c

@@ -89,13 +89,14 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
                      struct sockaddr *sa_out,
                      socklen_t len)
 {
+  memset(sa_out, 0, len);
+
   sa_family_t family = tor_addr_family(a);
   if (family == AF_INET) {
     struct sockaddr_in *sin;
     if (len < (int)sizeof(struct sockaddr_in))
       return 0;
     sin = (struct sockaddr_in *)sa_out;
-    memset(sin, 0, sizeof(struct sockaddr_in));
 #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
     sin->sin_len = sizeof(struct sockaddr_in);
 #endif
@@ -108,7 +109,6 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
     if (len < (int)sizeof(struct sockaddr_in6))
       return 0;
     sin6 = (struct sockaddr_in6 *)sa_out;
-    memset(sin6, 0, sizeof(struct sockaddr_in6));
 #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_LEN
     sin6->sin6_len = sizeof(struct sockaddr_in6);
 #endif
@@ -129,6 +129,9 @@ tor_addr_from_sockaddr(tor_addr_t *a, const struct sockaddr *sa,
 {
   tor_assert(a);
   tor_assert(sa);
+
+  memset(a, 0, sizeof(*a));
+
   if (sa->sa_family == AF_INET) {
     struct sockaddr_in *sin = (struct sockaddr_in *) sa;
     tor_addr_from_ipv4n(a, sin->sin_addr.s_addr);