Why is my packet reading program returning incorrect data?

Question

I have written a packet reader that uses libpcap to read a capture file. It reads the capture file and uploads captured data to a MySQL database. Sometimes it seems to work fine and others it returns invalid data (e.g. the source and destination ip will be the same, the tcp ports are all junk). I'm running this under a virtual RHEL 5. Here is my code (sorry if it's rather long or unnecessarily convoluted, this is my first attempt at this).

#include <arpa/inet.h>
#include <net/ethernet.h>

#include <netinet/ether.h>
#include <netinet/if_ether.h>
#include <netinet/in.h>
#include <netinet/ip.h>

#include "npc_tcp.h"
#include "npc_udp.h"

#include <ftw.h>
#include <pcap.h>
#include <stdio.h>
#include <sys/stat.h>
#include <regex.h>
#include <string.h>
#include <time.h>

const int IPTYPE_TCP = 6;
const int IPTYPE_UDP = 17;

struct cap_data {
  char ts[64];
  u_int16_t ether_type;
  u_int16_t proto;
  char *srcip;
  char *dstip;
  char *srcmac;
  u_int16_t srcport;
  u_int16_t dstport;
  u_int8_t flags;
  u_int capsize;
};

int main(int argc, char **argv) {

  //pcap
  struct cap_data data;
  struct pcap_pkthdr pkthdr;
  const u_char *packet;
  pcap_t *handle;
  char *fname = argv[1];
  printf("%s\n", fname);
  char errbuf[PCAP_ERRBUF_SIZE]; 
  handle = pcap_open_offline(fname, errbuf);
  char buf[1000];

  while (packet = pcap_next(handle, &pkthdr)) {

    int ether_flag;
    struct ether_header *ether;    
    u_short ether_type;           

    ether = (struct ether_header *) packet;
    data.ether_type = ntohs(ether->ether_type);

    ether_flag = 1;

    if (ether_flag) {
      if (data.ether_type == ETHERTYPE_IP) {
        struct ip *ip_hdr;
        u_int length = pkthdr.len;

        ip_hdr = (struct ip *)(packet + sizeof(struct ether_header));

      data.proto = ip_hdr->ip_p;
      data.dstip = inet_ntoa(ip_hdr->ip_dst);
      data.srcip = inet_ntoa(ip_hdr->ip_src);

      if (data.proto == IPTYPE_TCP) {
        struct tcphdr *tcp;
        tcp = (struct tcphdr*)(packet + sizeof(struct ether_header) + 
                sizeof(struct ip));

        data.srcport = tcp->th_sport;
        data.dstport = tcp->th_dport;

        printf("%s %u   %s %u\n\n", inet_ntoa(ip_hdr->ip_src), tcp->th_sport, inet_ntoa(ip_hdr->ip_dst), tcp->th_dport);

      } else if (data.proto == IPTYPE_UDP) {
        struct udphdr *udp;
        udp = (struct udphdr *)(packet + sizeof(struct ether_header) + 
                sizeof(struct ip));
        data.srcport = udp->uh_sport;
        data.dstport = udp->uh_dport;

        printf("%s %u   %s %u\n\n", inet_ntoa(ip_hdr->ip_src), udp->uh_sport, inet_ntoa(ip_hdr->ip_dst), udp->uh_dport);

        }
      }
    }
  }//while

  pcap_close(handle);

  return 0;
} 

UPDATE:

This outputs..

source ip      port   dest ip       port

66.68.236.207 30151     66.68.236.207 47873

172.22.162.235 60920    172.22.162.235 36175

67.207.28.150 23007     67.207.28.150 22038

172.22.162.235 60920    172.22.162.235 36175

67.207.28.151 22038     67.207.28.151 23007

65.55.87.43 20480       65.55.87.43 21764

67.207.28.150 23007     67.207.28.150 22038

The addresses should not be the same and the port numbers are wrong as well.

I have no idea where to even start looking for errors as my code (at least to me) looks correct. Any help, tips, or advice would be greatly appreciated. Thanks :)

Answer 1

Two problems:

1. inet_ntoa returns a pointer to a static buffer.
   When calling it twice within the same printf, it overwrites the same buffer. So you end up printing the same data.
   Use inet_ntop instead, and give each call a separate buffer.
   

2. You should use ntohs() to convert the ports to host-order before printing.

Answer 2

What happens if, right after calling pcap_open_offline(), you do

if (pcap_datalink(handle) != DLT_EN10MB) {
  fprintf(stderr, "This program handles only Ethernet captures\n");
  return 2;
}