[tor-bugs] #24081 [Core Tor/Torsocks]: Torsocks logging to a file can cause a crash or corrupt application files.

[Tor Bug Tracker & Wiki]

#24081: Torsocks logging to a file can cause a crash or corrupt application files.
-----------------------------------+---------------------
     Reporter:  cpwc               |      Owner:  dgoulet
         Type:  defect             |     Status:  new
     Priority:  High               |  Milestone:
    Component:  Core Tor/Torsocks  |    Version:
     Severity:  Major              |   Keywords:
Actual Points:                     |  Parent ID:
       Points:                     |   Reviewer:
      Sponsor:                     |
-----------------------------------+---------------------
 I ran into problems using torsocks with OpenSSH and tracked down several
 bugs in torsocks related to logging to a file. Here's a description, a
 test program to demonstrate the problem, and a patch.

 The initial problem was triggered by OpenSSH closing all file descriptors
 above the first three when it starts up. Some programs do that for good
 programming hygiene and security. See the BSD closefrom() function for
 background. When a program that does that is run with torsocks configured
 to log to a file, the log file descriptor is closed but torsocks doesn't
 notice. It keeps trying to write to the closed descriptor via the log file
 pointer as the program runs and doesn't detect the resulting write errors.
 If the file descriptor isn't reused then the log messages are silently
 lost. But if the descriptor is later opened for writing by the program
 then the torsocks log messages are written to the program's file!

 There is code intended to detect log write errors in log.c function
 _log_write(), but it doesn't work. The code looks at the return value from
 fprintf(), but fprintf() always returns success because the file is opened
 in buffered mode and the return value from a subsequent fflush() is
 ignored. So the write errors aren't detected.

 I added a call to setbuf() in log_init() to set the log file to unbuffered
 mode and that exposed further problems. Then log_write() sees fprintf()
 return an error and calls log_destroy() to clean up. log_destroy() calls
 fclose() to close the logconfig.fp file pointer and frees the malloced
 logconfig.filepath but doesn't zero them out, so subsequent logging calls
 still try to write to the closed fp, log_destroy() gets called again, the
 filepath is free()'d again (double free). Also the fclose() call goes to
 torsocks_fclose() which may generate another log message, causing a call
 loop and crash (SEGV due to stack overrun).

 The attached test_log_fd_close.c demonstrates the problem and file
 corruption. It simply closes file descriptors 3-5, creates some test files
 and closes them without writing to them, then verifies that they are in
 fact empty. Compile and link the program with the torsocks library and run
 it with:
         test_log_fd_close
 to see normal non-failing behavior, or run it with torsocks logging to a
 file:
         TORSOCKS_LOG_FILE_PATH=./log TORSOCKS_LOG_LEVEL=5
 test_log_fd_close
 to activate the bugs and see application file corruption.

 The attached log_fd_close_notify.patch fixes the problems. The changes
 are:
 - Add a function log_fd_close_notify() that can be called to notify the
 log system when an fd is being closed, so it can gracefully close out the
 log if necessary. Call it from torsocks_close().
 - Add a call to setbuf() in log_init() to make the torsocks log file
 unbuffered.
 - Don't call fclose() in log_destroy() and instead simply zero out the
 file pointer to stop future accesses to it. Also zero out the filepath
 pointer after freeing it to eliminate the potential for references to the
 freed pointer.
 - Eliminate another call to fclose() in log_init() just by reordering the
 existing code. A loop there is unlikely but it seems like a good change to
 make.

 I don't understand the torsocks test system to well enough to add
 test_log_fd_close.c to it. If it's worthwhile maybe someone else can do
 that.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24081>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online