s3: lib: nmbname: Ensure we limit the NetBIOS name correctly. CID: 1433607

Firstly, make the exit condition from the loop explicit (we must
never write into byte n, where n >= sizeof(name->name).

Secondly ensure exiting from the loop that n==MAX_NETBIOSNAME_LEN,
as this is the sign of a correct NetBIOS name encoding (RFC1002)
in order to properly read the NetBIOS name type (which is always
encoded in byte 16 == name->name[15]).

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>
Autobuild-User(master): David Disseldorp <ddiss@samba.org>
Autobuild-Date(master): Tue Nov 13 20:54:56 CET 2018 on sn-devel-144

diff --git a/source3/libsmb/nmblib.c b/source3/libsmb/nmblib.c
index ef6177e52096830066d7d3ea52e9539aa19b6d6e..727939575a74895addd9667d2bbedba7dfdfb03a 100644 (file)
--- a/source3/libsmb/nmblib.c
+++ b/source3/libsmb/nmblib.c
@@ -207,25 +207,33 @@ static int parse_nmb_name(char *inbuf,int ofs,int length, struct nmb_name *name)
                unsigned char c1,c2;
                c1 = ubuf[offset++]-'A';
                c2 = ubuf[offset++]-'A';
-               if ((c1 & 0xF0) || (c2 & 0xF0) || (n > sizeof(name->name)-1))
+               if ((c1 & 0xF0) || (c2 & 0xF0)) {
                        return(0);
+               }
+               if (n >= sizeof(name->name)) {
+                       return 0;
+               }
                name->name[n++] = (c1<<4) | c2;
                m -= 2;
        }
-       name->name[n] = 0;
-
-       if (n==MAX_NETBIOSNAME_LEN) {
-               /* parse out the name type, its always
-                * in the 16th byte of the name */
-               name->name_type = ((unsigned char)name->name[15]) & 0xff;
-
-               /* remove trailing spaces */
-               name->name[15] = 0;
-               n = 14;
-               while (n && name->name[n]==' ')
-                       name->name[n--] = 0;
+       /*
+        * RFC1002: For a valid NetBIOS name, exiting from the above,
+        * n *must* be MAX_NETBIOSNAME_LEN (16).
+        */
+       if (n != MAX_NETBIOSNAME_LEN) {
+               return 0;
        }
 
+       /* parse out the name type, its always
+        * in the 16th byte of the name */
+       name->name_type = ((unsigned char)name->name[15]) & 0xff;
+
+       /* remove trailing spaces */
+       name->name[15] = 0;
+       n = 14;
+       while (n && name->name[n]==' ')
+               name->name[n--] = 0;
+
        /* now the domain parts (if any) */
        n = 0;
        while (ubuf[offset]) {