linux-kernel - Re: [PATCH] isdnloop: NUL-terminate strings from userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOMGZ=E+zn-XZpfquP6jcCXxFY3CwN9j5X_iC6B_93o5222VHg@mail.gmail.com>
Date:	Mon, 31 Mar 2014 15:44:47 +0200
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org
Subject: Re: [PATCH] isdnloop: NUL-terminate strings from userspace

On 31 March 2014 15:36, Dan Carpenter <dan.carpenter@...cle.com> wrote:
> On Mon, Mar 31, 2014 at 02:56:07PM +0200, Vegard Nossum wrote:
>> Ping, Dave? Just making sure this doesn't fall through the cracks. I
>> don't see the patch applied anywhere yet and without this patch we
>> still have a valid security concern IMO.
>
> Gar.  No...  To recap:
>
>> On 7 March 2014 11:56, Vegard Nossum <vegard.nossum@...cle.com> wrote:
>> > Both the in-kernel and BSD strlcpy() require that the source string is
>> > NUL terminated.
>
> The *whole point* of strlcpy() is that the source string doesn't have to
> be NUL terminated.  The BSD man pages are trying to say that strlcpy()
> only works on C-strings as opposed to Vstr or other safer string
> implementions.

I read the BSD man page differently. Also, if you look at the actual
BSD implementation, it also scans the remaining buffer until it hits a
0.

I quote again: "for strlcpy() src must be NUL-terminated". It doesn't
get much clearer than that.

> There is a potential problem in the kernel implementation of strlcpy()
> because it does:
>
> lib/string.c
>    149  size_t strlcpy(char *dest, const char *src, size_t size)
>    150  {
>    151          size_t ret = strlen(src);
>    152
>    153          if (size) {
>    154                  size_t len = (ret >= size) ? size - 1 : ret;
>    155                  memcpy(dest, src, len);
>    156                  dest[len] = '\0';
>    157          }
>    158          return ret;
>    159  }
>
> The strlen() on line 151 could read beyond the end of the source buffer
> and if the memory wasn't mapped, it could Oops.
>
> That concern doesn't apply here because the source string is on stack
> memory and we will hit a NUL character before we hit unmapped memory.

As before, I agree that it's _likely_ we'll hit a 0 before hitting
unmapped memory, but I don't see at all that we have a guarantee of
it.


Vegard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/