All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tilman Schmidt <tilman@imap.cc>
To: Dan Carpenter <dan.carpenter@oracle.com>,
	Karsten Keil <isdn@linux-pingi.de>,
	netdev@vger.kernel.org
Subject: Re: [bug report] buffer overflow in isdn capi
Date: Tue, 03 Jun 2014 00:48:02 +0200	[thread overview]
Message-ID: <538CFF22.5020800@imap.cc> (raw)
In-Reply-To: <20140401154830.GA16759@mwanda>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello all,

on 01.04.2014 17:48, Dan Carpenter wrote:
> The command_2_index() function is buggy and leads to a buffer
> overflow. Does anyone know how to fix this?

AFAICS this is still unfixed.
As an easy fix I propose:

- --- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -203,14 +203,7 @@ static unsigned char *cpars[] =
 /*-------------------------------------------------------*/
 static unsigned command_2_index(unsigned c, unsigned sc)
 {
- -       if (c & 0x80)
- -               c = 0x9 + (c & 0x0f);
- -       else if (c <= 0x0f);
- -       else if (c == 0x41)
- -               c = 0x9 + 0x1;
- -       else if (c == 0xff)
- -               c = 0x00;
- -       return (sc & 3) * (0x9 + 0x9) + c;
+       return (sc & 3) * (0x9 + 0x9) + ((c & 0xf0) ? 0x9 : 0) + (c &
0x0f);
 }

 /*-------------------------------------------------------*/

This produces identical results to the current function for
all legal values of c (0x00..0x08, 0x41, 0x80..0x88, 0xff)
and guarantees that the result is <= 0x4e for all possible
inputs, whether legal or not. It also makes it clearer what
that function actually does.

Unless somebody points out a serious flaw with this I'll test it
and submit a formal patch.

Karsten's other concern about unhandled NULL pointers in the
mnames[] and cpars[] arrays should be addressed separately IMHO.

Regards,
Tilman

- -- 
Tilman Schmidt                              E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlOM/yIACgkQQ3+did9BuFuwMwCfVZq5Lx0P+ddhe/5WlxuO5zzp
VV4AoIn50I1wa4r4DtWfHFErMysgjsxr
=k/+h
-----END PGP SIGNATURE-----

      parent reply	other threads:[~2014-06-02 22:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-01 15:48 [bug report] buffer overflow in isdn capi Dan Carpenter
2014-04-01 16:25 ` Joe Perches
2014-04-02 16:46 ` Karsten Keil
2014-06-02 22:48 ` Tilman Schmidt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538CFF22.5020800@imap.cc \
    --to=tilman@imap.cc \
    --cc=dan.carpenter@oracle.com \
    --cc=isdn@linux-pingi.de \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.