diff --git a/docs/NOTES-python-gnupg-3.1-audit.html b/docs/NOTES-python-gnupg-3.1-audit.html new file mode 100644 index 0000000..fbd6e0d --- /dev/null +++ b/docs/NOTES-python-gnupg-3.1-audit.html @@ -0,0 +1,946 @@ + + + + +python-gnupg audit + + + + + + + + + + + + + +
+ +
+ +
+

python-gnupg audit

+ +

2013-02-01 Fri
+

+ +
+

Table of Contents

+
+ +
+
+ +
+

1 gnugp._main_()

+
+ + +
+ +
+

1.1 comments

+
+ +

L58 NullHandler?? see self.writepassphrase +L61 there nifty check for p3k +

+ +
+ +
+

1.2 def copydata(instream, outstream)    cleanup

+
+ +

copies data from one stream to another, 1024 bytes at a time. +

+
+ +
+

1.2.1 L79:    bad_logic

+
+ +

instream is apparently a file descriptor, but is not checked nor + encased in a try/except block. +

+
+ +
+ +
+

1.2.2 L78:    hanging_fd bad_logic

+
+ +

while True: loop, should be +

+with open(instream) as instrm:
+
+ +
+ +
+ +
+

1.2.3 L88:    bad_exception_handling

+
+ +
+except: 
+
+ +

should catch an IOError, or whatever specific error is raised for broken + pipes. +

+
+ +
+ +
+

1.3 def threadedcopydata(instream, outstream):

+
+ + +
+ +
+

1.3.1 L99:

+
+ +

this just wraps self.copydata in a thread +

+
+ +
+ +
+

1.4 def writepassphrase(stream, passphrase, encoding):    vuln cleanup

+
+ + +
+ +
+

1.4.1 L110:    writes_passphrase_to_disk

+
+ +

logger writes passphrase into debug log. this should be patched. +

+
+
+ +
+ +
+

2 class Verify(object)

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

3 class ImportResult(object)

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

4 class ListKeys(list):

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

5 class Crypt(Verify):

+
+ +

basic parsing class, no errors found +

+
+ +
+

5.1 def _init_(self, gpg)    cleanup

+
+ + +
+ +
+

5.1.1 L338    mro_conflict

+
+ + + + + +
Verify.__init__(self,gpg)
+
+ + +

+ should be changed to: +

+ + + +
super(Verify, self).__init__(gpg)
+
+ +
+
+
+ +
+ +
+

6 class GenKey(object)

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

7 class DeleteResult(object)

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

8 class Sign(object)

+
+ +

basic parsing class, no errors found +

+ +
+ +
+

9 class GPG(object)    exploitable

+
+ + +
+ +
+

9.1 L474:    cleanup

+
+ +
+cls.__doc__ 
+
+ +

should go directly underneath class signature +

+ +
+ +
+

9.1 def _init_(self, gpgbinary='gpg', gnupghome=None, verbose=False, useagent=False, keyring=None)    bug

+
+ + +
+ +
+

9.1.1 L494-495:    type_error

+
+ + + + + +
if gnupghome and not os.path.isdir(self.gnupghome):
+    os.makedirs(self.gnupghome,0x1C0)
+
+ + + +
In [20]: os.makedirs?
+Type:       function
+String Form:<function makedirs at 0x7f8ddeb6cc08>
+File:       /usr/lib/python2.7/os.py
+Definition: os.makedirs(name, mode=511)
+Docstring:
+makedirs(path [, mode=0777])
+Super-mkdir; create a leaf directory and all intermediate ones.
+Works like mkdir, except that any intermediate path segment (not
+just the rightmost) will be created if it does not exist.  This is
+recursive.
+
+setting mode=0x1c0 is equivalent to mode=hex(0700), which
+may cause bugs on some systems, see
+http://ubuntuforums.org/showthread.php?t=2044879
+
+this could be do to the complete lack of input validation in
+os.makedirs, and it's calling of the os.mkdir() built-in, which
+may vary depending on the python compilation:
+
+ + + +
Source:
+def makedirs(name, mode=0777):
+    """makedirs(path [, mode=0777])
+
+    Super-mkdir; create a leaf directory and all intermediate ones.
+    Works like mkdir, except that any intermediate path segment (not
+    just the rightmost) will be created if it does not exist.  This is
+    recursive.
+    """
+    head, tail = path.split(name)
+    if not tail:
+        head, tail = path.split(head)
+    if head and tail and not path.exists(head):
+        try:
+            makedirs(head, mode)
+        except OSError, e:
+            # be happy if someone already created the path
+            if e.errno != errno.EEXIST:
+                raise
+        if tail == curdir:           # xxx/newdir/. exists if xxx/newdir exists
+            return
+    mkdir(name, mode)
+
+ + +
+
+ +
+ +
+

9.2 def opensubprocess(self, args, passphrase=False)    vuln

+
+ + +
+ +
+

9.2.1 L515:    unvalidated_user_input

+
+ +
+cmd.extend(args)
+
+ + +

+ cmd is a list of strings, eventually joined with cmd=' '.join(cmd), and + the args are unvalidated in this function. Then this concatenation of args + is fed directly into subprocess.Popen(cmd, shell=True, stdin=PIPE, + stdout=PIPE, stderr=PIPE). THIS SHOULD BE PATCHED. +

+
+
+ +
+ +
+

9.3 def collectoutput(self, process, result, writer=None, stdin=None)

+
+ +

sends stdout to self.readdata() and stderr to self.readresponse() +

+
+ +
+ +
+

9.4 def handleio(self, args, file, result, passphrase=None, binary=False)    vuln cleanup

+
+ + +
+ +
+

9.4.1 L601:    unvalidated_user_input type_check_in_call

+
+ +
+p = self._open_subprocess(args, passphrase is not None)
+
+ + +

+ you shouldn't assign or type check in a function call +

+
+
+ +
+ +
+

9.5 def sign(self, message, **kwargs)    cleanup

+
+ + +
+ +
+

9.5.1 L617-619:    hanging_fd

+
+ +

calls self.makebinarystream(), which leaves the file descriptor for + the encoded message to be encrypted hanging between scopes. +

+
+
+ +
+ +
+

9.6 def signfile(self, file, keyid=None, passphrase=None, clearsign=True, detach=False, binary=False)    cleanup

+
+ + +
+ +
+

9.6.1 L632-635:    bad_logic

+
+ + + + +
if detach:
+    args.append("--detach-sign")
+elif clearsign:
+    args.append("--clearsign")
+
+ + +

+ the logic here allows that if a user erroneously specifies both options, + rather than doing what the system gnupg would do (that is, do –clearsign, + and ignore the –attach-sign), python-gnupg would ignore both. +

+
+ +
+ +
+

9.6.2 L626-641:

+
+ +

input 'args' into self.opensubprocess() is defined as static strings. +

+
+
+ +
+ +
+

9.7 def verify(self, data):    cleanup

+
+ + +
+ +
+

9.7.1 L668-670:    hanging_fd

+
+ +

same hanging file descriptor problem as in self.sign() +

+
+
+ +
+ +
+

9.8 def verifyfile(self, file, datafilename=None)    vuln cleanup

+
+ + +
+ +
+

9.8.1 L683:    hanging_fd

+
+ +

more potentially hanging file descriptors… +

+ +
+ +
+

9.8.2 L684:    hanging_fd

+
+ +

oh look, another hanging file descriptor. imagine that. +

+ +
+ +
+

9.8.3 L690:    unvalidated_user_input

+
+ +
+args.append('"%s"' % data_filename)
+
+ +

well, there's the exploit. see included POC script. +

+
+
+ +
+ +
+

9.9 def importkeys(self, keydata)    vuln

+
+ + +
+ +
+

9.9.1 L749:    unvalidated_user_input

+
+ +

this function could potentially allow an attacker with a GPG exploit to + use it, because it passes key generation parameter directly into the + internal packet parsers of GPG. however, without a GPG exploit for one of + the GPG packet parsers (for explanation of GPG packets look into pgpdump), + this function alone is not exploitable. +

+
+
+ +
+ +
+

9.10 def recievekeys(self, keyserver, *keyids)    vuln

+
+ + +
+ +
+

9.10.1 L770:    unvalidated_user_input

+
+ +
+args.extend(keyids)
+
+ + +
+
+ +
+ +
+

9.11 def exportkeys(self, keyids, secret=False)    vuln

+
+ + +
+ +
+

9.11.1 L795-796:    unvalidated_user_input

+
+ +

args problem again. exploitable though parameter ``keyids``. +

+
+
+ +
+ +
+

9.12 def listkeys(self, secret=False)

+
+ + +
+ +
+

9.12.1 L827:

+
+ +

args is static string. +

+
+
+ +
+ +
+

9.13 def genkey(self, input)    cleanup

+
+ + +
+ +
+

9.13.1 L864:

+
+ +

args, passed to self.handleio(), which in turn passes args directly to + Popen(), is set to a static string. this function is halfway okay, though + it really could be more careful with the ``input`` parameter. +

+
+
+ +
+ +
+

9.14 def genkeyinput(self, **kwargs)    vuln

+
+ + +
+ +
+

9.14.1 L981-983:    unvalidated_user_input

+
+ +

this function could potentially allow an attacker with a GPG exploit to + use it, because it passes key generation parameter directly into the + internal packet parsers of GPG. however, without a GPG exploit for one of + the GPG packet parsers (for explanation of GPG packets look into pgpdump), + this function alone is not exploitable. +

+
+
+ +
+ +
+

9.15 def encryptfile(self, file, recipiencts, sign=None, …)    vuln

+
+ + +
+ +
+

9.15.1 L939:    unvalidated_user_input

+
+ +

several of the inputs to this function are unvalidated, turned into + strings, and passed to Popen(). exploitable. +

+
+
+ +
+ +
+

9.16 def encrypt(self, data, recipients, **kwargs):    vuln

+
+ + +
+ +
+

9.16.1 L997:    unvalidated_user_input

+
+ +

exploitable, passes kwargs to self.encryptfile() +

+
+
+ +
+ +
+

9.17 def decrypt(self, message **kwargs):    vuln

+
+ + +
+ +
+

9.17.1 L1003:    unvalidated_user_input

+
+ +

kwargs are passed to self.decryptfile(), unvalidated, making this + function also exploitable +

+
+
+ +
+ +
+

9.18 def decryptfile(self, file, alwaystrust=False, passphrase=None, output=None)    vuln

+
+ + +
+ +
+

9.18.1 L1013:    unvalidated_user_input

+
+ +

unvalidated user input: this function is also exploitable +

+
+
+
+ +
+ +
+

10 POC

+
+ +

CANNOT INCLUDE FILE ../python-gnupg-0.3.1/python-gnupg-exploit.py +

+
+
+ +
+

Date: 2013-02-01 Fri

+

Author: isis

+

isis@leap.se

+

Org version 7.9.2 with Emacs version 24

+Validate XHTML 1.0 + +
+ +