GitLab

connection_handle_listener_read():

...
  if (conn->socket_family == AF_INET || conn->socket_family == AF_INET6) {
    tor_addr_t addr;
    uint16_t port;
    if (check_sockaddr(remote, remotelen, LOG_INFO)<0) {
      log_info(LD_NET,
               "accept() returned a strange address; trying getsockname().");
      remotelen=sizeof(addrbuf);
      memset(addrbuf, 0, sizeof(addrbuf));
      if (getsockname(news, remote, &remotelen)<0) {
        int e = tor_socket_errno(news);
        log_warn(LD_NET, "getsockname() for new connection failed: %s",
                 tor_socket_strerror(e));
      } else {
        if (check_sockaddr((struct sockaddr*)addrbuf, remotelen,
                              LOG_WARN) < 0) {
          log_warn(LD_NET,"Something's wrong with this conn. Closing it.");
          tor_close_socket(news);
          return 0;
        }
      }
    }
...

If both check_sockaddr() and getsockname()` fail, we continue with a corrupted remote (and sockaddr), instead of closing the socket and returning 0.

I'm not sure if this is a bug, but it feels weird, since if the second check_sockaddr() fails, we close the socket and return 0, which is what I feel that should happen.

I'm also not sure how someone can make both check_sockaddr() and getsockname()fail, and I'm not sure howremote` would have to look in this case or what would happen afterwards.

All in all, is the validation here intended to be like this?