[tor-commits] [tor/release-0.3.5] Don't pass a NULL into a %s when logging client auth file load failure

[nickm@xxxxxxxxxxxxxx]

commit ff5584034361154de381874e1a24d0a421aa0631
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Fri May 10 17:47:43 2019 -0400

    Don't pass a NULL into a %s when logging client auth file load failure
    
    Fortunately, in 0.3.5.1-alpha we improved logging for various
    failure cases involved with onion service client auth.
    
    Unfortunately, for this one, we freed the file right before logging
    its name.
    
    Fortunately, tor_free() sets its pointer to NULL, so we didn't have
    a use-after-free bug.
    
    Unfortunately, passing NULL to %s is not defined.
    
    Fortunately, GCC 9.1.1 caught the issue!
    
    Unfortunately, nobody has actually tried building Tor with GCC 9.1.1
    before. Or if they had, they didn't report the warning.
    
    Fixes bug 30475; bugfix on 0.3.5.1-alpha.
---
 changes/bug30475            | 4 ++++
 src/feature/hs/hs_service.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/changes/bug30475 b/changes/bug30475
new file mode 100644
index 000000000..839597b88
--- /dev/null
+++ b/changes/bug30475
@@ -0,0 +1,4 @@
+  o Minor bugfixes ():
+    - Avoid a GCC 9.1.1 warning (and possible crash depending on libc
+      implemenation) when failing to load a hidden service client authorization
+      file.  Fixes bug 30475; bugfix on 0.3.5.1-alpha.
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 828ca1da4..402929036 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -1259,16 +1259,16 @@ load_client_keys(hs_service_t *service)
     client_key_file_path = hs_path_from_filename(client_keys_dir_path,
                                                  filename);
     client_key_str = read_file_to_str(client_key_file_path, 0, NULL);
-    /* Free immediately after using it. */
-    tor_free(client_key_file_path);
 
     /* If we cannot read the file, continue with the next file. */
     if (!client_key_str)  {
       log_warn(LD_REND, "Client authorization file %s can't be read. "
                         "Corrupted or verify permission? Ignoring.",
                client_key_file_path);
+      tor_free(client_key_file_path);
       continue;
     }
+    tor_free(client_key_file_path);
 
     client = parse_authorized_client(client_key_str);
     /* Wipe and free immediately after using it. */



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits