diff options
Diffstat (limited to 'docs/NOTES-python-gnupg-3.1-audit.org')
| -rw-r--r-- | docs/NOTES-python-gnupg-3.1-audit.org | 232 |
1 files changed, 0 insertions, 232 deletions
diff --git a/docs/NOTES-python-gnupg-3.1-audit.org b/docs/NOTES-python-gnupg-3.1-audit.org deleted file mode 100644 index b80fb39..0000000 --- a/docs/NOTES-python-gnupg-3.1-audit.org +++ /dev/null @@ -1,232 +0,0 @@ -#+TITLE: python-gnupg audit -#+AUTHOR: isis -#+EMAIL: isis@leap.se -#+DATE: 2013-02-01 Fri -#+DESCRIPTION: -#+KEYWORDS: -#+LANGUAGE: en -#+OPTIONS: H:3 num:t toc:t \n:nil @:t ::t |:t ^:t -:t f:t *:t <:t -#+OPTIONS: TeX:t LaTeX:t skip:nil d:nil todo:t pri:nil tags:not-in-toc -#+INFOJS_OPT: view:nil toc:2 ltoc:t mouse:underline buttons:0 path:http://orgmode.org/org-info.js -#+EXPORT_SELECT_TAGS: export -#+EXPORT_EXCLUDE_TAGS: noexport -#+LINK_UP: -#+LINK_HOME: -#+XSLT: - -[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: - - #+BEGIN_SRC python - Verify.__init__(self,gpg) - #+END_SRC - - should be changed to: - - #+BEGIN_SRC python - super(Verify, self).__init__(gpg) - #+END_SRC -* 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: - - #+BEGIN_SRC python - if gnupghome and not os.path.isdir(self.gnupghome): - os.makedirs(self.gnupghome,0x1C0) - #+END_SRC - - #+BEGIN_EXAMPLE - 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: - #+END_EXAMPLE - - #+BEGIN_SRC python - 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) - #+END_SRC - -** 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: - #+BEGIN_SRC python - if detach: - args.append("--detach-sign") - elif clearsign: - args.append("--clearsign") - #+END_SRC - - 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 -#+INCLUDE: "../python-gnupg-0.3.1/python-gnupg-exploit.py" python |