summaryrefslogtreecommitdiff
path: root/docs/NOTES-python-gnupg-3.1-audit.org
blob: b80fb393986da52e2e918d132a7b030b8b4e10b4 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
#+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