From 008b773834b26d9e38c2edb4f2d6ab532dde7a94 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 17 Oct 1997 02:56:23 +0000 Subject: fix the handling of negative name query responses and the handling of packets with no answer section in general. The fix has 2 parts: 1) set ans_name to the name we queried if nmb->answers == NULL 2) check for nmb->answers == NULL in several other places where we currently check for nmb->answers->data While doing this, I noticed there are lots of places in our nmbd code where we make assumptions about the packets being well formed. Someone could easily implement a denial of service attack on nmbd by sending a packet that causes a null pointer dereference. Does anyone feel like going through the code and adding checks? Probably the best solution is to have a single function that "validates" a packet, making sure that all the required fields are there. This will be a bit tricky as what fields are required varies a lot between packets. A first pass would be a function that prints "SUSPECT PACKET" when it hits a packet that it suspects does not have a required field (or the field is badly formatted), then we could use this on a live system to find any cases we've missed. Any takers? (This used to be commit e02c21b0b8e3ed6f2d294458160c4f632af67ed3) --- source3/nameservresp.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/source3/nameservresp.c b/source3/nameservresp.c index 8de90113fb..deb56c0850 100644 --- a/source3/nameservresp.c +++ b/source3/nameservresp.c @@ -59,7 +59,7 @@ static void response_name_release(struct nmb_name *ans_name, DEBUG(4,("response name release received\n")); - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) { /* IMPORTANT: see expire_netbios_response_entries() */ @@ -113,9 +113,9 @@ static void response_name_reg(struct nmb_name *ans_name, */ if ( ((d != wins_subnet) && (nmb->header.rcode == 6) && strequal(myworkgroup, name) && (type == 0x1b)) || - (nmb->header.rcode == 0 && nmb->answers->rdata)) + (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata)) #else - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) #endif { /* IMPORTANT: see expire_netbios_response_entries() */ @@ -154,7 +154,7 @@ static void response_server_check(struct nmb_name *ans_name, we sent to - rather than reading the response. */ - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) putip((char*)&send_ip,&nmb->answers->rdata[2]); else { @@ -269,7 +269,8 @@ static void response_name_status_check(struct in_addr ip, struct nmb_name name; fstring serv_name; - if (interpret_node_status(d,nmb->answers->rdata, + if (nmb->answers && + interpret_node_status(d,nmb->answers->rdata, &name,0x20,serv_name,ip,bcast)) { if (*serv_name) @@ -311,7 +312,7 @@ static void response_name_query_register(struct nmb_packet *nmb, return; } - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) { /* we had sent out a name query to the current owner of a name because someone else wanted it. now they @@ -388,7 +389,7 @@ static void response_name_query_sync(struct nmb_packet *nmb, return; } - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) { int nb_flags = nmb->answers->rdata[0]; struct in_addr found_ip; @@ -464,7 +465,7 @@ for %s\n", nmb->header.rcode == 0 ? "success" : "failure", /* Check the name is correct and ip address returned is our own. If it is then we just remove the response record. */ - if (name_equal(&n->name, ans_name) && (nmb->header.rcode == 0) && (nmb->answers->rdata)) + if (name_equal(&n->name, ans_name) && (nmb->header.rcode == 0) && nmb->answers && (nmb->answers->rdata)) { struct in_addr found_ip; @@ -576,7 +577,7 @@ static BOOL response_problem_check(struct response_record *n, { if (n->num_msgs > 1) { - if (nmb->header.rcode == 0 && nmb->answers->rdata) + if (nmb->header.rcode == 0 && nmb->answers && nmb->answers->rdata) { int nb_flags = nmb->answers->rdata[0]; @@ -822,27 +823,25 @@ void response_netbios_packet(struct packet_struct *p) DEBUG(2,("expected on subnet %s. hmm.\n", inet_ntoa(d->bcast_ip))); } - if (nmb->answers == NULL) - { - /* hm. the packet received was a response, but with no answer. wierd! */ - DEBUG(2,("NMB packet response from %s (bcast=%s) - UNKNOWN\n", - inet_ntoa(p->ip), BOOLSTR(bcast))); - return; - } + if (nmb->answers == NULL) { + /* if there is no name is the response then the name is the one + we queried on */ + ans_name = &n->name; + } else { + ans_name = &nmb->answers->rr_name; + debug_rr_type(nmb->answers->rr_type); + } - ans_name = &nmb->answers->rr_name; DEBUG(3,("response for %s from %s(%d) (bcast=%s)\n", namestr(ans_name), inet_ntoa(p->ip), p->port, BOOLSTR(bcast))); - debug_rr_type(nmb->answers->rr_type); - n->num_msgs++; /* count number of responses received */ n->repeat_count = 0; /* don't resend: see expire_netbios_packets() */ debug_state_type(n->state); /* problem checking: multiple responses etc */ - if (response_problem_check(n, nmb, ans_name->name)) + if (nmb->answers && response_problem_check(n, nmb, ans_name->name)) return; /* now deal with the current state */ -- cgit