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...

233 lines
9.9 KiB
Org Mode
Raw Blame History

#+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
Reference in New Issue View Git Blame Copy Permalink