tags:

views:

91

answers:

2

hello i am using windows and ANSI-c , i have an application that sniffs data from network card then decodes it and sent to and other application via UDP. the problem is my application starts with fine working but after some time its all variables int, float and strings are corrupted i have checked it many times but can't be able to dig out the problem. my this very application works very fine when i take data straight to my ip with out sniffing data from network.

can any one suggest me how to tackle this problem, any testing tool or else that can help

sorry for weak English and thanx in advance.

//mehtod for start sniffing the raw port
void StartSniffing(SOCKET sniffer)
{
    unsigned char *Buffer = ( unsigned char *)malloc(65536);
    int iPacketLength;
    if (Buffer == NULL)
    {
        printf("malloc() failed.\n");
        return;
    }//end if

    iPacketLength = recvfrom(sniffer,(char *)Buffer,65536,0,0,0); 

    if(iPacketLength > 0 && iPacketLength <50)
        ProcessPacket(Buffer, iPacketLength);
    else
        printf( "recvfrom() failed.\n");

    free(Buffer);
}//end method
//*****************************************************************************************
// method for pecket processing filters udp
void ProcessPacket(unsigned char* Buffer, int Size)
{
    char srtPacketSourceIp[16]={'\0'};
    char strPacketDestinationIp[16] ={'\0'} ;
    strcpy_s(srtPacketSourceIp,inet_ntoa(source.sin_addr));
    strcpy_s(strPacketDestinationIp,inet_ntoa(dest.sin_addr));

    int a=strcmp(srtPacketSourceIp, strSourceIp );
    int b=strcmp(strPacketDestinationIp,strDestinationIp);

    iphdr = (IPV4_HDR *)Buffer;


    switch (iphdr->ip_protocol) //Check the Protocol and do accordingly...
    {
        case 1: //ICMP Protocol
            //PrintIcmpPacket(Buffer,Size);
            break;
        case 2: //IGMP Protocol
            break;
        case 6: //TCP Protocol
            //PrintTcpPacket(Buffer,Size);
            break;
        case 17: //UDP Protocol
            ++udp;          
            if(!strcmp(srtPacketSourceIp, strSourceIp )&& !strcmp(strPacketDestinationIp,strDestinationIp))
                PrintUdpPacket(Buffer,Size);
            break;
        default: //Some Other Protocol like ARP etc.
            break;
    }//end switch

}//end method
//*****************************************************************************************
//mehtod to print ip header for displaing data
void PrintIpHeader (unsigned char* Buffer, int Size)
{
    unsigned short iphdrlen;
    iphdr = (IPV4_HDR *)Buffer;
    iphdrlen = iphdr->ip_header_len*4;
    memset(&source, 0, sizeof(source));
    source.sin_addr.s_addr = iphdr->ip_srcaddr;
    memset(&dest, 0, sizeof(dest));
    dest.sin_addr.s_addr = iphdr->ip_destaddr;
    /*printf("\n");
    printf("IP Header\n");
    printf(" |-IP Version : %d\n",(unsigned int)iphdr->ip_version);
    printf(" |-IP Header Length : %d DWORDS or %d Bytes\n",(unsigned int)iphdr->ip_header_len);
    printf(" |-Type Of Service : %d\n",(unsigned int)iphdr->ip_tos);
    printf(" |-IP Total Length : %d Bytes(Size of Packet)\n",ntohs(iphdr->ip_total_length));
    printf(" |-Identification : %d\n",ntohs(iphdr->ip_id));
    printf(" |-Reserved ZERO Field : %d\n",(unsigned int)iphdr->ip_reserved_zero);
    printf(" |-Dont Fragment Field : %d\n",(unsigned int)iphdr->ip_dont_fragment);
    printf(" |-More Fragment Field : %d\n",(unsigned int)iphdr->ip_more_fragment);
    printf(" |-TTL : %d\n",(unsigned int)iphdr->ip_ttl);
    printf(" |-Protocol : %d\n",(unsigned int)iphdr->ip_protocol);
    printf(" |-Checksum : %d\n",ntohs(iphdr->ip_checksum));
    printf(" |-Source IP : %s\n",inet_ntoa(source.sin_addr));
    printf(" |-Destination IP : %s\n",inet_ntoa(dest.sin_addr));*/

}//end method
//*****************************************************************************************
//method to print udp packet for displaying data
void PrintUdpPacket(unsigned char *Buffer,int Size)
{
    unsigned short iphdrlen;
    iphdr = (IPV4_HDR *)Buffer;
    iphdrlen = iphdr->ip_header_len*4;
    udpheader = (UDP_HDR *)(Buffer + iphdrlen);
    /*printf("\n\n***********************UDP Packet*************************\n");
    PrintIpHeader(Buffer,Size);
    printf("\nUDP Header\n");
    printf(" |-Source Port : %d\n",ntohs(udpheader->source_port));
    printf(" |-Destination Port : %d\n",ntohs(udpheader->dest_port));
    printf(" |-UDP Length : %d\n",ntohs(udpheader->udp_length));
    printf(" |-UDP Checksum : %d\n",ntohs(udpheader->udp_checksum));
    printf("\n");
    printf("IP Header\n");
    PrintData(Buffer,iphdrlen);
    printf("UDP Header\n");
    PrintData(Buffer+iphdrlen,sizeof(UDP_HDR));
    printf("Data Payload\n");
    PrintData(Buffer+iphdrlen+sizeof(UDP_HDR),(Size - sizeof(UDP_HDR) - iphdr->ip_header_len*4));*/
    memcpy(strRecievedDump,Buffer+iphdrlen+sizeof(UDP_HDR),(Size - sizeof(UDP_HDR) - iphdr->ip_header_len*4));
    DumpDecoderAndSender();//this just decodes and sends the data
    //printf("\n###########################################################");

}//end method
//*****************************************************************************************
//method to print data
void PrintData (unsigned char* data , int Size)
{
    for(int i=0 ; i < Size ; i++)
    {
        if( i!=0 && i%16==0) //if one line of hex printing is complete...
        {
            printf(" ");

            for(int j=i-16 ; j<i ; j++)
            {
                if(data[j]>=32 && data[j]<=128)
                    printf("%c",(unsigned char)data[j]); //if its a number or alphabet
                else 
                    printf("."); //otherwise print a dot
            }//end for
            printf("\n");

        }//end if

        if(i%16==0)
            printf(" ");

        printf(" %02X",(unsigned int)data[i]);

        if( i==Size-1) //print the last spaces
        {
            for(int j=0;j<15-i%16;j++)
                (" "); //extra spaces

            printf(" ");

            for(int j=i-i%16 ; j<=i ; j++)
            {
                if(data[j]>=32 && data[j]<=128)
                    printf("%c",(unsigned char)data[j]);
                else 
                    printf(".");
            }//end if
            printf("\n");
        }//end if
    }//end for loop
}//end method   
+1  A: 

Another suggestion is to run your program under a memory checker, e.g. valgrind.

Peter G.
valgrind is for linux is there any for windows
moon
+1  A: 

Just 2 or 3 points

Your StartSniffing function has no loop so I suppose it is called for each received packet, in this case allocating/freeing for every call is wastefull. Use a stack array or let the caller allocate the buffer once and pass it to the callee. Allocation can be costly due to locking.

In ProcessPacket you declare 2 arrays that you initialize and then fill with strcpy. Remove the initializers they are not necessary (they are hidden memset calls).

You then declare 2 variable a and b with the result of the comparisons of Ip addresses, so far so good but then in case 17 you don't use them and recheck with strcmp which is not so good. So either reuse the a and b vars or remove them altogether.

In PrintIpHeader and other avoid these printf call fests. printf is costly so it is better to have only one big call than 10 smaller ones. Here an exapmple of what I mean.

printf("IP Header\n"
       " |-IP Version : %u\n"
       " |-IP Header Length : %u DWORDS or %u Bytes\n"    /* 2 %u but only 1 parameter??? */
       " |-Type Of Service : %u\n"
       " |-IP Total Length : %u Bytes(Size of Packet)\n"
       " |-Identification : %u\n"
       " |-Reserved ZERO Field : %u\n"
       " |-Dont Fragment Field : %u\n"
       " |-More Fragment Field : %u\n"
       " |-TTL : %d\n"
       " |-Protocol : %d\n"
       " |-Checksum : %d\n"
       " |-Source IP : %s\n"
       " |-Destination IP : %s\n", (unsigned int)iphdr->ip_version
                                 , (unsigned int)iphdr->ip_header_len, 9999 /* Added 9999 because the original code was buggy */
                                 , (unsigned int)iphdr->ip_tos
                                 , ntohs(iphdr->ip_total_length)
                                 , ntohs(iphdr->ip_id)
                                 , (unsigned int)iphdr->ip_reserved_zero
                                 , (unsigned int)iphdr->ip_dont_fragment
                                 , (unsigned int)iphdr->ip_more_fragment
                                 , (unsigned int)iphdr->ip_ttl 
                                 , (unsigned int)iphdr->ip_protocol
                                 , ntohs(iphdr->ip_checksum)
                                 , inet_ntoa(source.sin_addr)
                                 , inet_ntoa(dest.sin_addr));  

Notice that the %u is the format for unsigned int not %d and your format in the 2nd printf is false. You should compile with warnings enabled, gcc would have warned about that.

tristopia