Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netsmb From FreeBSD:



details:   https://anonhg.NetBSD.org/src/rev/8f50996208b5
branches:  trunk
changeset: 826881:8f50996208b5
user:      christos <christos%NetBSD.org@localhost>
date:      Tue Oct 03 15:27:10 2017 +0000

description:
>From FreeBSD:

netsmb: Fix buggy/racy smb_strdupin()

smb_strdupin() tried to roll a copyin() based strlen to allocate a buffer
and then blindly copyin that size.  Of course, a malicious user program
could simultaneously manipulate the buffer, resulting in a non-terminated
string being copied.

Later assumptions in the code rely upon the string being nul-terminated.

Just use copyinstr() and drop the racy sizing.

PR:             222687
Reported by:    Meng Xu <meng.xu AT gatech.edu>
Security:       possible local DoS
Sponsored by:   Dell EMC Isilon

diffstat:

 sys/netsmb/smb_subr.c |  23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diffs (46 lines):

diff -r 0623b8682dc5 -r 8f50996208b5 sys/netsmb/smb_subr.c
--- a/sys/netsmb/smb_subr.c     Tue Oct 03 11:02:36 2017 +0000
+++ b/sys/netsmb/smb_subr.c     Tue Oct 03 15:27:10 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: smb_subr.c,v 1.38 2017/07/28 14:37:27 riastradh Exp $  */
+/*     $NetBSD: smb_subr.c,v 1.39 2017/10/03 15:27:10 christos Exp $   */
 
 /*
  * Copyright (c) 2000-2001 Boris Popov
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: smb_subr.c,v 1.38 2017/07/28 14:37:27 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: smb_subr.c,v 1.39 2017/10/03 15:27:10 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -114,20 +114,15 @@
 char *
 smb_strdupin(char *s, size_t maxlen)
 {
-       char *p, bt;
-       size_t len = 0;
+       char *p;
+       int error;
 
-       for (p = s; ;p++) {
-               if (copyin(p, &bt, 1))
-                       return NULL;
-               len++;
-               if (maxlen && len > maxlen)
-                       return NULL;
-               if (bt == 0)
-                       break;
+       p = malloc(maxlen + 1, M_SMBSTR, M_WAITOK);
+       error = copyinstr(s, p, maxlen + 1, NULL);
+       if (error) {
+               free(p, M_SMBSTR);
+               return NULL;
        }
-       p = malloc(len, M_SMBSTR, M_WAITOK);
-       copyin(s, p, len);
        return p;
 }
 



Home | Main Index | Thread Index | Old Index