Hi Zed

On 2017-01-05 16:53:44, Zed Pobre wrote:
> On Wed, Jan 04, 2017 at 01:14:19PM +0100, Sebastian Ramacher wrote:
> > On 2017-01-03 13:07:38, Zed Pobre wrote:
> > > Despite that, I think I agree that paramiko needs to change.  The
> > > problem is that this is a stable distribution, and the patch that
> > > causes this problem, used to fix #849495, is really just attempting to
> > > prevent bad usage by other programs, not inherently fixing a security
> > > flaw.  In addition, the CTR component isn't actually dangerous, just
> > > "confusing".
> > >
> > > I propose that you remove the following from src/block_template.c:
> > >
> > > ++      if (IVlen != 0 && mode == MODE_CTR)
> > > ++      {
> > > ++              PyErr_Format(PyExc_ValueError,
> > > ++                      "CTR mode needs counter parameter, not IV");
> > > ++              return NULL;
> > > ++      }
> >
> > No, dropping thas would open up the vulnerability again. For jessie the
> > exception was downgraded to a warning and IVlen set to 0.
> >
> > For wheezy LTS I sent the updated patch to Chris Lamb (CCed). I'd expect an
> > update there soon.
> 
> Unfortunately, python-crypto 2.6-4+deb7u5 does not fix the problem.

It was fixed in 2-6-4+deb7u6.

> I
> would like to reiterate that as far as CTR is concerned *there is no
> vulnerability* according to all the discussion I have read on it thus
> far -- at worst, it could be considered "confusing" and lead to
> third-party misuse of the library, no examples of which in the wild
> have yet been presented.

Then perhaps the discussions you read are wrong or incomplete. Maybe there is no
exploitable code out there using CTR mode, but it can be used to trigger the
very same buffer overflow as with ECB. Here is the interesting part of AES.new
before applying IVlen checks for ECB and CTR:

        if (IVlen != BLOCK_SIZE && mode != MODE_ECB && mode != MODE_CTR)
        {
                PyErr_Format(PyExc_ValueError,
                             "IV must be %i bytes long", BLOCK_SIZE);
                return NULL;
        }

This is the only IVlen check and does not have any effect in ECB or CTR mode.

        if (mode == MODE_CTR) {
                if (counter == NULL) {
                        PyErr_SetString(PyExc_TypeError,
                                        "'counter' keyword parameter is 
required with CTR mode");
                        return NULL;
#ifdef IS_PY3K
                } else if (PyObject_HasAttr(counter, 
PyUnicode_FromString("__PCT_CTR_SHORTCUT__"))) {
#else
                } else if (PyObject_HasAttrString(counter, 
"__PCT_CTR_SHORTCUT__")) {
#endif
                        counter_shortcut = 1;
                } else if (!PyCallable_Check(counter)) {
                        PyErr_SetString(PyExc_ValueError, 
                                        "'counter' parameter must be a callable 
object");
                        return NULL;
                }
        } else {
                if (counter != NULL) {
                        PyErr_SetString(PyExc_ValueError, 
                                        "'counter' parameter only useful with 
CTR mode");
                        return NULL;
                }
        }

In CTR mode, only the presence of counter is checked.

        memset(new->IV, 0, BLOCK_SIZE);
        memset(new->oldCipher, 0, BLOCK_SIZE);
        memcpy(new->IV, IV, IVlen);

The last memcpy now causes a buffer overflow in case of MODE_CTR and MODE_ECB
since IVlen is never checked and completely controlled by a potential adversary.

Cheers
-- 
Sebastian Ramacher

Attachment: signature.asc
Description: PGP signature

Reply via email to