Re: [PATCH] Correction to patch to Nping BETA packet recv handler

["Luis M." <luis.mgarc () gmail com>]

Hi Jack,

I've been having a look at your patch. I just added a few more safe
checks in getDestAddrFromICMPPacket() to ensure the ip->ip_hl does not
contain a bogus value that could produce a segfault, but apart from that
it looks great!  I've just applied it.

Thank you very much, Jack.

Best regards,

Luis.




Jack Grahl wrote:
> There was a mistake in the previous patch - instead of ip->len it should of course be ip->hl.
> Jack
>
> ---------
> new patch
> ---------
>
> --- nping-0.1BETA2/nping.cc    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/nping.cc    2009-08-22 02:42:32.000000000 +0100
> @@ -1661,6 +1661,7 @@
>   static struct timeval prevtime;
>   NpingTarget *trg=NULL;
>   u16 *prt=NULL;
> + u8 proto=NULL;
>  
>   outPrint(DBG_4, "nping_event_handler(): Received callback of type %s with status %s",
>                    nse_type2str(type), nse_status2str(status));
> @@ -1725,27 +1726,54 @@
>              else
>                  outError(QT_1, "Unsupported EtherType %02X", *ethtype);
>  
> -            outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> -            if( o.getVerbosity() >= VB_3 )
> -                luis_hdump((char*)packet, packetlen);
> -            prevtime=pcaptime;
> -
> -            /* Statistics */
> -            o.stats.addRecvPacket(packetlen);
> -            trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> -
> -            if( o.getMode()==TCP || o.getMode()==UDP){
> -                if(trg != NULL){
> -                    prt=getSrcPortFromIPPacket((u8*)packet, packetlen);
> -                    if( prt!=NULL )              
> -                        trg->setProbeRecvTCP(*prt, 0);
> -                }
> -            }else if (o.getMode()==ICMP){
> -                if(trg != NULL){
> -                    trg->setProbeRecvICMP(0, 0);
> -                }
> -            }
> -       break;
> +        proto = getProtoFromIPPacket((u8*)packet, packetlen);
> +
> +        if (proto == IPPROTO_UDP || proto == IPPROTO_TCP){
> +          /* for UDP/TCP we print out and update the global total straight away
> +         since we know that pcap only found packets from connections that we 
> +         opened */
> +          outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> +          if( o.getVerbosity() >= VB_3 )
> +        luis_hdump((char*)packet, packetlen);
> +          prevtime=pcaptime;
> +          
> +          /* Statistics */
> +          o.stats.addRecvPacket(packetlen);
> +
> +          /* then we check for a target and a port and do the individual stats */
> +          trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> +          
> +          if(trg != NULL){
> +        prt=getSrcPortFromIPPacket((u8*)packet, packetlen);
> +        if( prt!=NULL )              
> +          trg->setProbeRecvTCP(*prt, 0);
> +          }
> +
> +        }else if (proto==IPPROTO_ICMP){
> +          /* we look for a target based on first src addr and second the dest addr of 
> +         the packet header which is returned in the ICMP packet */
> +          trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> +          if(trg == NULL){
> +        trg=o.targets.findTarget( getDestAddrFromICMPPacket((u8*)packet, packetlen));
> +          }
> +
> +          /* In the case of ICMP we only do any printing and stats if we 
> +         found a target - otherwise it could be a packet that is nothing 
> +         to do with us */
> +          if(trg!=NULL){
> +          outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> +          if( o.getVerbosity() >= VB_3 )
> +        luis_hdump((char*)packet, packetlen);
> +          prevtime=pcaptime;
> +          
> +          o.stats.addRecvPacket(packetlen);
> +          
> +          trg->setProbeRecvICMP(0, 0);
> +          }
> +          
> +        }
> +        
> +        break;
>  
>         /* In theory we should never get these kind of events in this handler
>          * because no code schedules them */  
> diff -u -r nping-0.1BETA2/utils_net.cc nping-build2/utils_net.cc
> --- nping-0.1BETA2/utils_net.cc    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/utils_net.cc    2009-08-22 02:39:10.000000000 +0100
> @@ -1360,12 +1360,65 @@
>  } /* End of getSrcSockAddrFromPacket() */
>  
>  
> +struct sockaddr_storage *getDestAddrFromICMPPacket(u8 *pkt, size_t pktLen){
>  
> +  static struct sockaddr_storage ss;
> +  struct sockaddr_in *s_ip4=(struct sockaddr_in *)&ss;
> +  struct ip *i4=(struct ip*)pkt;
> +  memset(&ss, 0, sizeof(struct sockaddr_storage));
>  
> +  if(pkt==NULL || pktLen < 48)
> +    return NULL;
> +
> +  if( i4->ip_v == 4 ){
> +    s_ip4->sin_family=AF_INET;
> +    memcpy(&(s_ip4->sin_addr.s_addr), pkt + 24 + i4->ip_hl * 4, 4);
> +
> +    return &ss;
> +  }
> +  else {
> +    return NULL;
> +  }
> +
> +}
> +  /* End of getDestAddrFromICMPPacket */
>  
>  
>  
>  
> +
> +u8 *getUDPheaderLocation(u8 *pkt, size_t pktLen){
> +
> +struct ip *i4=(struct ip*)pkt;
> +  if(pkt==NULL || pktLen < 40)
> +    return NULL;
> +
> +  /* Packet is IPv4 */
> +  if( i4->ip_v == 4 ){
> +    if (i4->ip_p == IPPROTO_UDP) { 
> +        if( pktLen >= ((size_t)(i4->ip_hl*4 + 8)) ) /* We have a full IP+UDP packet */
> +            return pkt+(i4->ip_hl*4);    
> +    }
> +    else
> +        return NULL;
> +  }
> +  /* Packet is IPv6 */
> +  else if(i4->ip_v == 6 ){
> +    if(pktLen<40 + 8 ) /* We possible have a full IPv6+UDP packet */
> +        return NULL;
> +    if( pkt[6] == IPPROTO_UDP ) /* Next Header is TCP? */
> +        return pkt+40;
> +    else /* Extension headers not supported, return NULL TODO: support it? */
> +        return NULL;
> +  }
> +  else{
> +      return NULL;
> +  }
> +
> +  return NULL;
> +}
> +  /* end of getUDPheaderLocation */
> +
>  u8 *getTCPheaderLocation(u8 *pkt, size_t pktLen){
>  
>  struct ip *i4=(struct ip*)pkt;
> @@ -1401,6 +1454,30 @@
>  
>  
>  
> +/* Returns the IP protocol of the packet or -1 in case of failure */
> +u8 getProtoFromIPPacket(u8 *pkt, size_t pktLen){
> +struct ip *i4=(struct ip*)pkt;
> +static u8 proto;
> +
> +  if(pkt==NULL || pktLen < 28)
> +    return -1;
> +
> +  /* Packet is IPv4 */
> +  if( i4->ip_v == 4 ){
> +    proto = i4->ip_p;
> +    return proto;
> +  }
> +
> +  /* Packet is IPv6 */
> +  else if(i4->ip_v == 6 ){
> +    proto = pkt[6];
> +    return proto;
> +  }
> +
> +  return -1;
> +
> +} /* End of getProtoFromIPPacket() */
> +
>  
>  
>  /** @warning Returns pointer to an internal static buffer
> @@ -1409,19 +1486,23 @@
>  
>  static u16 port;
>  u16 *pnt=NULL;
> -u8 *tcp=NULL;
> +u8 *header=NULL;
>  
> -  if(pkt==NULL || pktLen < 40)
> +  if(pkt==NULL || pktLen < 28)
>      return NULL;
>  
> -  if((tcp=getTCPheaderLocation(pkt, pktLen))==NULL)
> -    return NULL;
> +  if((header=getTCPheaderLocation(pkt, pktLen))==NULL){
> +    if ((header=getUDPheaderLocation(pkt, pktLen))==NULL)
> +      return NULL;
> +    
> +  }
>  
> -  pnt=(u16*)&(tcp[0]);
> +  pnt=(u16*)&(header[0]);
>    port= ntohs(*pnt);
> -
> +  
>    return &port;
>    
> +  
>  } /* End of getSrcPortFromIPPacket() */
>  
>  
> diff -u -r nping-0.1BETA2/utils_net.h nping-build2/utils_net.h
> --- nping-0.1BETA2/utils_net.h    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/utils_net.h    2009-08-22 02:13:45.000000000 +0100
> @@ -129,7 +129,10 @@
>  int print_dnet_interface(const struct intf_entry *entry, void *arg) ;
>  int print_interfaces_dnet();
>  struct sockaddr_storage *getSrcSockAddrFromIPPacket(u8 *pkt, size_t pktLen);
> +struct sockaddr_storage *getDestAddrFromICMPPacket(u8 *pkt, size_t pktLen);
> +u8 *getUDPheaderLocation(u8 *pkt, size_t pktLen);
>  u8 *getTCPheaderLocation(u8 *pkt, size_t pktLen);
> +u8 getProtoFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getSrcPortFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getDstPortFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getDstPortFromTCPHeader(u8 *pkt, size_t pktLen);
>
>
>       
>
> _______________________________________________
> Sent through the nmap-dev mailing list
> http://cgi.insecure.org/mailman/listinfo/nmap-dev
> Archived at http://SecLists.Org
>
>   


_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org