Unsignedio
/
python-gnupg
mirror of https://github.com/markqvist/python-gnupg.git
1
0
Fork 0
Code Issues Releases Wiki Activity
python-gnupg/docs/NOTES-python-gnupg-3.1-audi...

9.9 KiB
Raw Blame History

python-gnupg audit

-- mode: org; epa-file-encrypt-to: ("lunar@anargeek.net") --

[2013-02-01 Fri]

gnugp.__main__()

comments

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

def _copy_data(instream, outstream)   cleanup

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

L79:   bad_logic

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

L78:   hanging_fd bad_logic

while True: loop, should be 

with open(instream) as instrm:

L88:   bad_exception_handling 

except:

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

def _threaded_copy_data(instream, outstream):

L99:

this just wraps self._copy_data in a thread

def _write_passphrase(stream, passphrase, encoding):   vuln cleanup

L110:   writes_passphrase_to_disk

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

class Verify(object)

basic parsing class, no errors found

class ImportResult(object)

basic parsing class, no errors found

class ListKeys(list):

basic parsing class, no errors found

class Crypt(Verify):

basic parsing class, no errors found

def __init__(self, gpg)   cleanup

L338   mro_conflict 

Verify.__init__(self,gpg)

should be changed to:

super(Verify, self).__init__(gpg)

class GenKey(object)

basic parsing class, no errors found

class DeleteResult(object)

basic parsing class, no errors found

class Sign(object)

basic parsing class, no errors found

class GPG(object)   exploitable

L474:   cleanup 

cls.__doc__

should go directly underneath class signature

def __init__(self, gpgbinary='gpg', gnupghome=None, verbose=False, use_agent=False, keyring=None)   bug

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)

def _open_subprocess(self, args, passphrase=False)   vuln

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.

def _collect_output(self, process, result, writer=None, stdin=None)

sends stdout to self._read_data() and stderr to self._read_response()

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

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

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

L617-619:   hanging_fd

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

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

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.

L626-641:

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

def verify(self, data):   cleanup

L668-670:   hanging_fd

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

def verify_file(self, file, data_filename=None)   vuln cleanup

L683:   hanging_fd

more potentially hanging file descriptors…

L684:   hanging_fd

oh look, another hanging file descriptor. imagine that.

L690:   unvalidated_user_input 

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

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

def import_keys(self, key_data)   vuln

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.

def recieve_keys(self, keyserver, *keyids)   vuln

L770:   unvalidated_user_input 

args.extend(keyids)

def export_keys(self, keyids, secret=False)   vuln

L795-796:   unvalidated_user_input

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

def list_keys(self, secret=False)

L827:

args is static string.

def gen_key(self, input)   cleanup

L864:

args, passed to self._handle_io(), 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.

def gen_key_input(self, **kwargs)   vuln

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.

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

L939:   unvalidated_user_input

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

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

L997:   unvalidated_user_input

exploitable, passes kwargs to self.encrypt_file()

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

L1003:   unvalidated_user_input

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

def decrypt_file(self, file, always_trust=False, passphrase=None, output=None)   vuln

L1013:   unvalidated_user_input

unvalidated user input: this function is also exploitable

POC