[RESOLVED] Using npcap libs to save packet causes memory fault

Hi,
I'm writing a command line utility to process a particular type of packet using a filter. The utility has worked every time, no problem at all but when I added code to save the packet to a .pcap file BEFORE decoding and displaying fields, I get a memory access error like this:

Exception thrown: read access violation.
**ptr_Field** was 0x20798A99000.

It happens most of the time, not always. But, if I save the packet to file AFTER I process it, it works no problem. It seems that the npcap API call "pcap_dump(dumpfile, header, pkt_data);" is overwriting memory.

Any came across this before?

UPDATE: I commented out the line "pcap_dump(dumpfile, header, pkt_data);" which left the lines to open the file "dumpfile = pcap_dump_open(adhandle, argv[2]);" and close the file "pcap_close(adhandle);" and I still get the momery error, so even just opening a .pcap file causes the problem.

I know the easy solution is to save the packet after I process it, but this shouldn't happen.

> so even just opening a .pcap file causes the problem.
No, opening the file exposes the problem.

The problem already existed, it's just that you managed to avoid it killing your program up until now.

That it hasn't crashed beforehand is just dumb luck.

You can make a start by running valgrind, and fixing any memory errors (buffer overruns, use after free).

An example.

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *get_mem(size_t len) {
    char *p = malloc(len);
    return p;
}

void update(char *p, const char *word) {
    strcpy(p, word);
}

void free_mem(char *p) {
    free(p);
}

int main ( ) {
    char *p = get_mem(5);
    update(p,"hello");  // actually 6 bytes
    free_mem(p);
    return 0;
}

---

Code:

---

$ # compile with -g for debug
$ gcc -g foo.c

# program runs 'fine', but clearly there is a problem in the code
$ ./a.out

# valgrind finds it!
$ valgrind ./a.out
==72080== Memcheck, a memory error detector
==72080== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==72080== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==72080== Command: ./a.out
==72080==
==72080== Invalid write of size 1
==72080==    at 0x484EE8E: strcpy (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==72080==    by 0x1091D5: update (foo.c:11)
==72080==    by 0x109227: main (foo.c:20)
==72080==  Address 0x4aa1045 is 0 bytes after a block of size 5 alloc'd
==72080==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==72080==    by 0x1091A4: get_mem (foo.c:6)
==72080==    by 0x10920D: main (foo.c:19)
==72080==
==72080==
==72080== HEAP SUMMARY:
==72080==    in use at exit: 0 bytes in 0 blocks
==72080==  total heap usage: 1 allocs, 1 frees, 5 bytes allocated
==72080==
==72080== All heap blocks were freed -- no leaks are possible
==72080==
==72080== For lists of detected and suppressed errors, rerun with: -s
==72080== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

---

For each issue, valgrind gives you two stack traces
- where the problem was detected
- where the memory was originally allocated.

For each one, you decide
- was too much being copied in
- was not enough allocated to begin with.

Thanks for the reply salem_c. I'm teaching myself C++ using Visual Studio in Windows and looking at your sample code, do I need to allocate memory for the captured packet? The sample Npcap code didn't do that and that's how I started off.
It looks like my next bit of research is memory reservation/allocation, would that be right?

Quote:

---

do I need to allocate memory for the captured packet

---

That depends upon what the used api documentation says. You don't provide any code for us to have a look at.

> The sample Npcap code didn't do that and that's how I started off.
Some URLs to say the library you're using and it's documentation and examples would be really useful about now.

I allocated memory to a byte array and copied the packet data to the array which worked no problem and referenced the byte array instead when decoding the packet, but the same error occurred.
I've come up with a solution for this. I replaced the function that decoded the packet to a function that just printed out the hex values of every byte in the packet and that worked ok so I reckoned it must be the command.
The command I used referenced a pointer to a byte and converted the byte and following byte to a short int. I changed this to reading the 2 individual bytes and doing simple arithmetic to get the short int equivalent.

Memory allocation error:
TTL = (short*)ptrCurrent_Field;
shortTTL = ntohs(*TTL);

Works:
shortTTL = ptrCurrent_Field[0] * 0x100 + ptrCurrent_Field[1];

I used the 'error causing' code at several other places throughout the code and it works ok so I'm still confused. Maybe receiving a packet from the interface during a live capture treats the packet data as a stream of bytes but reading a .pcap file has a more defined structure.

I thought about posting code, but I thought there would have been too much to go through. Your post about memory allocation did get me thinking through testing more thoroughly and that led to the fix, so thanks a lot salem_c, you did help me :)
I haven't fully understood why the fix worked, but that's for me to do more research. That's half the fun of learning ;)

ntohs() requires a u_short param (unsigned) and not a short (signed). So try:

Code:

---

shortTTL = ntohs((u_short)*ptrCurrent_Field);

---

Assuming that ptrCurrent_Field isn't of void*. What is the type of ptrCurrent-Field?

ptrCurrent_Field is type u_char* I was using that as a general pointer as a reference to the next field in the packet to decode and casting it to be a pointer to a short int. I got it from a forum port (I can't find where but it wasn't in CodeGuru, maybe that was my mistake :)

Then

Code:

---

shortTTL = ntohs(*ptrCurrent_Field);

---

assuming that ptrCurrent_Field is not NULL and is valid.

Sorry my bad/mistake. See Salam's post below. I misread the previous post for the type of ptrCurrent_Field :blush::o

Thanks 2kaud. I tried that, but unfortunately I didn't get the right result. *ptrCurrent_Field points to a byte with value 0x0f, the next byte is d3, but after the command "shortTTL = ntohs(*ptrCurrent_Field);" shortTTL contains 0x0f00 rather than 0x0fd3. The same with the line "shortTTL = ntohs((u_short)*ptrCurrent_Field);"

Assuming you don't come unstuck with a bus error due to bad pointer alignment (not an issue on x86), you want something like

shortTTL = ntohs(*((u_short*)ptrCurrent_Field));

The red part casts your ptrCurrent_Field to be a pointer to a u_short, which is then dereferenced to produce a u_short.

Quote:

---

Originally Posted by salem_c

Assuming you don't come unstuck with a bus error due to bad pointer alignment (not an issue on x86), you want something like

shortTTL = ntohs(*((u_short*)ptrCurrent_Field));

The red part casts your ptrCurrent_Field to be a pointer to a u_short, which is then dereferenced to produce a u_short.

---

Quote:

---

ptrCurrent_Field is type u_char*

---

Watch endiness!

> Watch endiness!
That's what the ntoh function deals with.

Assuming the documentation for that field says "this ushort is in network byte order", then ntohs is just going to do it's thing and all is good.

The in parameter requires a 16-bit number in TCP/IP network byte order. As the cast is from char* to u-short* the order of bytes ptrCurrent_Field[0] and ptrCurrent_Field[1] is important and isn't dealt with by ntoh().

shortTTL = ntohs(*((u_short*)ptrCurrent_Field));

...worked and it looks neater than the workaround I used. Thanks a lot to all who replied. It gives me a pointer (pun intended) for something to learn more about.