Found by the CodeNomicon test suites at the SNIA plugfest.
http://www.codenomicon.com/
If an invalid NetBIOS session request is received the code in name_len() in
libsmb/nmblib.c can hit an assert.
Re-write name_len() and name_extract() to use "buf/len" pairs and
always limit reads.
Jeremy.
int matching_len_bits(unsigned char *p1, unsigned char *p2, size_t len);
void sort_query_replies(char *data, int n, struct in_addr ip);
char *name_mangle(TALLOC_CTX *mem_ctx, char *In, char name_type);
-int name_extract(char *buf,int ofs, fstring name);
-int name_len(char *s1);
+int name_extract(unsigned char *buf,size_t buf_len, unsigned int ofs, fstring name);
+int name_len(unsigned char *s1, size_t buf_len);
/* The following definitions come from libsmb/nterr.c */
files_struct *fsp);
bool check_fsp_ntquota_handle(connection_struct *conn, struct smb_request *req,
files_struct *fsp);
-void reply_special(struct smbd_server_connection *sconn, char *inbuf);
+void reply_special(struct smbd_server_connection *sconn, char *inbuf, size_t inbuf_len);
void reply_tcon(struct smb_request *req);
void reply_tcon_and_X(struct smb_request *req);
void reply_unknown_new(struct smb_request *req, uint8 type);
{
char *p;
int len = 4;
+ int namelen = 0;
char *tmp;
/* 445 doesn't have session request */
}
p = cli->outbuf+len;
- memcpy(p, tmp, name_len(tmp));
- len += name_len(tmp);
+ namelen = name_len((unsigned char *)tmp, talloc_get_size(tmp));
+ if (namelen > 0) {
+ memcpy(p, tmp, namelen);
+ len += namelen;
+ }
TALLOC_FREE(tmp);
/* and my name */
}
p = cli->outbuf+len;
- memcpy(p, tmp, name_len(tmp));
- len += name_len(tmp);
+ namelen = name_len((unsigned char *)tmp, talloc_get_size(tmp));
+ if (namelen > 0) {
+ memcpy(p, tmp, namelen);
+ len += namelen;
+ }
TALLOC_FREE(tmp);
/* send a session request (RFC 1002) */
/****************************************************************************
Interpret the weird netbios "name" into a unix fstring. Return the name type.
+ Returns -1 on error.
****************************************************************************/
-static int name_interpret(char *in, fstring name)
+static int name_interpret(unsigned char *buf, size_t buf_len,
+ unsigned char *in, fstring name)
{
+ unsigned char *end_ptr = buf + buf_len;
int ret;
- int len = (*in++) / 2;
+ unsigned int len;
fstring out_string;
- char *out = out_string;
+ unsigned char *out = (unsigned char *)out_string;
*out=0;
- if (len > 30 || len<1)
- return(0);
+ if (in >= end_ptr) {
+ return -1;
+ }
+ len = (*in++) / 2;
+
+ if (len<1) {
+ return -1;
+ }
while (len--) {
+ if (&in[1] >= end_ptr) {
+ return -1;
+ }
if (in[0] < 'A' || in[0] > 'P' || in[1] < 'A' || in[1] > 'P') {
*out = 0;
return(0);
*out = ((in[0]-'A')<<4) + (in[1]-'A');
in += 2;
out++;
+ if (PTR_DIFF(out,out_string) >= sizeof(fstring)) {
+ return -1;
+ }
}
ret = out[-1];
out[-1] = 0;
-#ifdef NETBIOS_SCOPE
- /* Handle any scope names */
- while(*in) {
- *out++ = '.'; /* Scope names are separated by periods */
- len = *(unsigned char *)in++;
- StrnCpy(out, in, len);
- out += len;
- *out=0;
- in += len;
- }
-#endif
pull_ascii_fstring(name, out_string);
return(ret);
Find a pointer to a netbios name.
****************************************************************************/
-static char *name_ptr(char *buf,int ofs)
+static unsigned char *name_ptr(unsigned char *buf, size_t buf_len, unsigned int ofs)
{
- unsigned char c = *(unsigned char *)(buf+ofs);
+ unsigned char c = 0;
+
+ if (ofs > buf_len || buf_len < 1) {
+ return NULL;
+ }
+ c = *(unsigned char *)(buf+ofs);
if ((c & 0xC0) == 0xC0) {
- uint16 l = RSVAL(buf, ofs) & 0x3FFF;
+ uint16 l = 0;
+
+ if (ofs > buf_len - 1) {
+ return NULL;
+ }
+ l = RSVAL(buf, ofs) & 0x3FFF;
+ if (l > buf_len) {
+ return NULL;
+ }
DEBUG(5,("name ptr to pos %d from %d is %s\n",l,ofs,buf+l));
return(buf + l);
} else {
/****************************************************************************
Extract a netbios name from a buf (into a unix string) return name type.
+ Returns -1 on error.
****************************************************************************/
-int name_extract(char *buf,int ofs, fstring name)
+int name_extract(unsigned char *buf, size_t buf_len, unsigned int ofs, fstring name)
{
- char *p = name_ptr(buf,ofs);
- int d = PTR_DIFF(p,buf+ofs);
+ unsigned char *p = name_ptr(buf,buf_len,ofs);
name[0] = '\0';
- if (d < -50 || d > 50)
- return(0);
- return(name_interpret(p,name));
+ if (p == NULL) {
+ return -1;
+ }
+ return(name_interpret(buf,buf_len,p,name));
}
/****************************************************************************
Return the total storage length of a mangled name.
+ Returns -1 on error.
****************************************************************************/
-int name_len(char *s1)
+int name_len(unsigned char *s1, size_t buf_len)
{
/* NOTE: this argument _must_ be unsigned */
unsigned char *s = (unsigned char *)s1;
- int len;
+ int len = 0;
+ if (buf_len < 1) {
+ return -1;
+ }
/* If the two high bits of the byte are set, return 2. */
- if (0xC0 == (*s & 0xC0))
+ if (0xC0 == (*s & 0xC0)) {
+ if (buf_len < 2) {
+ return -1;
+ }
return(2);
+ }
/* Add up the length bytes. */
for (len = 1; (*s); s += (*s) + 1) {
len += *s + 1;
- SMB_ASSERT(len < 80);
+ if (len > buf_len) {
+ return -1;
+ }
}
return(len);
/*
* NetBIOS session request, keepalive, etc.
*/
- reply_special(conn, (char *)inbuf);
+ reply_special(conn, (char *)inbuf, nread);
goto done;
}
}
/****************************************************************************
- Reply to a (netbios-level) special message.
+ Reply to a (netbios-level) special message.
****************************************************************************/
-void reply_special(struct smbd_server_connection *sconn, char *inbuf)
+void reply_special(struct smbd_server_connection *sconn, char *inbuf, size_t inbuf_size)
{
int msg_type = CVAL(inbuf,0);
int msg_flags = CVAL(inbuf,1);
- fstring name1,name2;
- char name_type1, name_type2;
-
/*
* We only really use 4 bytes of the outbuf, but for the smb_setlen
* calculation & friends (srv_send_smb uses that) we need the full smb
*/
char outbuf[smb_size];
- *name1 = *name2 = 0;
-
memset(outbuf, '\0', sizeof(outbuf));
smb_setlen(outbuf,0);
switch (msg_type) {
case 0x81: /* session request */
+ {
+ /* inbuf_size is guarenteed to be at least 4. */
+ fstring name1,name2;
+ int name_type1, name_type2;
+ int name_len1, name_len2;
+
+ *name1 = *name2 = 0;
if (sconn->nbt.got_session) {
exit_server_cleanly("multiple session request not permitted");
SCVAL(outbuf,0,0x82);
SCVAL(outbuf,3,0);
- if (name_len(inbuf+4) > 50 ||
- name_len(inbuf+4 + name_len(inbuf + 4)) > 50) {
+
+ /* inbuf_size is guaranteed to be at least 4. */
+ name_len1 = name_len((unsigned char *)(inbuf+4),inbuf_size - 4);
+ if (name_len1 <= 0 || name_len1 > inbuf_size - 4) {
+ DEBUG(0,("Invalid name length in session request\n"));
+ return;
+ }
+ name_len2 = name_len((unsigned char *)(inbuf+4+name_len1),inbuf_size - 4 - name_len1);
+ if (name_len2 <= 0 || name_len2 > inbuf_size - 4 - name_len1) {
DEBUG(0,("Invalid name length in session request\n"));
return;
}
- name_type1 = name_extract(inbuf,4,name1);
- name_type2 = name_extract(inbuf,4 + name_len(inbuf + 4),name2);
+
+ name_type1 = name_extract((unsigned char *)inbuf,
+ inbuf_size,(unsigned int)4,name1);
+ name_type2 = name_extract((unsigned char *)inbuf,
+ inbuf_size,(unsigned int)(4 + name_len1),name2);
+
+ if (name_type1 == -1 || name_type2 == -1) {
+ DEBUG(0,("Invalid name type in session request\n"));
+ return;
+ }
+
DEBUG(2,("netbios connect: name1=%s0x%x name2=%s0x%x\n",
name1, name_type1, name2, name_type2));
sconn->nbt.got_session = true;
break;
+ }
case 0x89: /* session keepalive request
(some old clients produce this?) */
}
}
-static void filter_request(char *buf)
+static void filter_request(char *buf, size_t buf_len)
{
int msg_type = CVAL(buf,0);
int type = CVAL(buf,smb_com);
- fstring name1,name2;
unsigned x;
+ fstring name1,name2;
+ int name_len1, name_len2;
+ int name_type1, name_type2;
if (msg_type) {
/* it's a netbios special */
- switch (msg_type) {
+ switch (msg_type)
case 0x81:
/* session request */
- name_extract(buf,4,name1);
- name_extract(buf,4 + name_len(buf + 4),name2);
+ /* inbuf_size is guaranteed to be at least 4. */
+ name_len1 = name_len((unsigned char *)(buf+4),
+ buf_len - 4);
+ if (name_len1 <= 0 || name_len1 > buf_len - 4) {
+ DEBUG(0,("Invalid name length in session request\n"));
+ return;
+ }
+ name_len2 = name_len((unsigned char *)(buf+4+name_len1),
+ buf_len - 4 - name_len1);
+ if (name_len2 <= 0 || name_len2 > buf_len - 4 - name_len1) {
+ DEBUG(0,("Invalid name length in session request\n"));
+ return;
+ }
+
+ name_type1 = name_extract((unsigned char *)buf,
+ buf_len,(unsigned int)4,name1);
+ name_type2 = name_extract((unsigned char *)buf,
+ buf_len,(unsigned int)(4 + name_len1),name2);
+
+ if (name_type1 == -1 || name_type2 == -1) {
+ DEBUG(0,("Invalid name type in session request\n"));
+ return;
+ }
+
d_printf("sesion_request: %s -> %s\n",
name1, name2);
if (netbiosname) {
/* replace the destination netbios
* name */
memcpy(buf+4, mangled,
- name_len(mangled));
+ name_len((unsigned char *)mangled,
+ talloc_get_size(mangled)));
TALLOC_FREE(mangled);
}
}
- }
return;
}
SIVAL(buf, smb_vwv11, x);
break;
}
-
}
/****************************************************************************
d_printf("client closed connection\n");
exit(0);
}
- filter_request(packet);
+ filter_request(packet, len);
if (!send_smb(s, packet)) {
d_printf("server is dead\n");
exit(1);