[tor-commits] [arm/release] Checking the auth cookie's size before reading

[atagar at torproject.org]

commit b05787fb09764fe49473e82ca787dc6e92d5f0d5
Author: Damian Johnson <atagar at torproject.org>
Date:   Wed Oct 26 21:56:03 2011 -0700

    Checking the auth cookie's size before reading
    
    Adding a check that the authentication cookie is 32 bytes before sending its
    contents to the control port. This is to prevent a malicious socket from
    tricking us into reading them arbitrary file content. Tested by hardcoding an
    alternative file as being the cookie and confirming that this makes arm abort
    initialization. Caught by rransom.
    
    https://trac.torproject.org/projects/tor/ticket/4305
---
 src/cli/controller.py  |    4 ++++
 src/cli/headerPanel.py |   10 ++++++++++
 src/starter.py         |   13 +++++++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/src/cli/controller.py b/src/cli/controller.py
index aebc6f3..4bef5ec 100644
--- a/src/cli/controller.py
+++ b/src/cli/controller.py
@@ -550,6 +550,10 @@ class TorManager:
     
     if authType == TorCtl.AUTH_TYPE.COOKIE:
       try:
+        authCookieSize = os.path.getsize(authValue)
+        if authCookieSize != 32:
+          raise IOError("authentication cookie '%s' is the wrong size (%i bytes instead of 32)" % (authValue, authCookieSize))
+        
         torctlConn.authenticate(authValue)
         torTools.getConn().init(torctlConn)
       except Exception, exc:
diff --git a/src/cli/headerPanel.py b/src/cli/headerPanel.py
index 2c32af8..bf8dbdb 100644
--- a/src/cli/headerPanel.py
+++ b/src/cli/headerPanel.py
@@ -146,6 +146,12 @@ class HeaderPanel(panel.Panel, threading.Thread):
         cli.popups.showMsg("Unable to reconnect (socket '%s' doesn't exist)" % self._config["startup.interface.socket"], 3)
       
       if not torctlConn and allowPortConnection:
+        # TODO: This has diverged from starter.py's connection, for instance it
+        # doesn't account for relative cookie paths or multiple authentication
+        # methods. We can't use the starter.py's connection function directly
+        # due to password prompts, but we could certainly make this mess more
+        # manageable.
+        
         try:
           ctlAddr, ctlPort = self._config["startup.interface.ipAddress"], self._config["startup.interface.port"]
           tmpConn, authType, authValue = TorCtl.TorCtl.preauth_connect(ctlAddr, ctlPort)
@@ -153,6 +159,10 @@ class HeaderPanel(panel.Panel, threading.Thread):
           if authType == TorCtl.TorCtl.AUTH_TYPE.PASSWORD:
             authValue = cli.popups.inputPrompt("Controller Password: ")
             if not authValue: raise IOError() # cancel reconnection
+          elif authType == TorCtl.TorCtl.AUTH_TYPE.COOKIE:
+            authCookieSize = os.path.getsize(authValue)
+            if authCookieSize != 32:
+              raise IOError("authentication cookie '%s' is the wrong size (%i bytes instead of 32)" % (authValue, authCookieSize))
           
           tmpConn.authenticate(authValue)
           torctlConn = tmpConn
diff --git a/src/starter.py b/src/starter.py
index 625000c..8deb500 100644
--- a/src/starter.py
+++ b/src/starter.py
@@ -243,6 +243,19 @@ def _torCtlConnect(controlAddr="127.0.0.1", controlPort=9051, passphrase=None, i
         if pathSuffix.startswith("/"): pathSuffix = pathSuffix[1:]
         
         conn._cookiePath = os.path.join(pathPrefix, pathSuffix)
+      
+      # Abort if the file isn't 32 bytes long. This is to avoid exposing
+      # arbitrary file content to the port.
+      #
+      # Without this a malicious socket could, for instance, claim that
+      # '~/.bash_history' or '~/.ssh/id_rsa' was its authentication cookie to
+      # trick us into reading it for them with our current permissions.
+      #
+      # https://trac.torproject.org/projects/tor/ticket/4305
+      
+      authCookieSize = os.path.getsize(conn._cookiePath)
+      if authCookieSize != 32:
+        raise IOError("authentication cookie '%s' is the wrong size (%i bytes instead of 32)" % (conn._cookiePath, authCookieSize))
     
     conn.authenticate(authValue)
     return conn