Fix niclist.exe crash when writing to string returned by PACKET.DLL (#179)

Seemingly Npcap returns a read-only string and `niclist.exe` tries to
modify (tokenize) it using `strtrok()`.
That results in the crash: #161

Probably related to
https://github.com/the-tcpdump-group/libpcap/pull/949.

Here is my try at fixing this.
For successful modification I propose to use a stack-allocated copy
instead of the original (const) version string.

Now
[packetWin7/Dll/Packet32.cpp#L159](https://github.com/nmap/npcap/blob/a41bc6a/packetWin7/Dll/Packet32.cpp#L159)
seems to initialize `const char PacketLibraryVersion[]` from the define
`WINPCAP_VER_STRING`, which seems to be of arbitrary length:
```
__declspec(dllexport) const char PacketLibraryVersion[] = WINPCAP_VER_STRING; 
```

Let's search for the longest string present in their repo:
```
$ git log -u version.h | awk 'BEGIN { FPAT="(([^ \t]+)?(\"[^\"]+\")?)+" } /^.*define.+WINPCAP_VER_STRING.+[0-9]/ { gsub(/"/, "", $NF); print $NF }' | sort -Vu | while read; do printf "%4s    %s\n" ${#REPLY} "${REPLY}"; done | sort -n
   4    0.01
   4    0.03
   4    0.04
   4    0.05
   4    0.06
   4    0.07
   4    0.08
   4    0.09
   4    0.10
   4    0.11
   4    0.78
   4    0.80
   4    0.81
   4    0.82
   4    0.83
   4    0.84
   4    0.85
   4    0.86
   4    0.90
   4    0.91
   4    0.92
   4    0.93
   4    0.94
   4    0.95
   4    0.96
   4    0.97
   4    0.98
   4    1.00
   4    1.10
   4    1.20
   4    1.30
   4    1.31
   4    1.40
   4    1.50
   4    1.55
   4    1.60
   4    1.70
   4    1.71
   4    1.72
   4    1.73
   4    1.74
   4    1.75
   4    1.76
   4    1.77
   4    1.78
   5    0.991
   5    0.992
   5    0.993
   5    0.994
   5    0.995
   5    0.996
   5    0.997
   6    0.9981
   6    0.9982
   6    0.9983
   6    0.9984
   6    0.9985
   6    0.9986
   6    0.9987
   6    0.9988
   6    0.9989
   6    0.9990
   6    0.9991
   6    0.9992
   6    0.9993
   6    0.9994
   6    0.9995
   6    0.9996
   6    0.9997
   7    0.08 r8
   7    0.08 r9
   7    0.09 r2
   7    0.09 r3
   7    0.09 r4
   7    0.09 r5
   7    0.09 r6
   7    0.09 r7
   7    0.09 r8
   7    0.09 r9
   7    0.10 r2
   7    0.10 r3
   7    0.10 r4
   7    0.10 r5
   7    0.10 r6
   7    0.10 r7
   7    0.10 r8
   7    0.10 r9
   7    0.78 r2
   7    0.78 r3
   7    0.78 r4
   7    0.78 r5
   7    0.99-r1
   7    0.99-r2
   7    0.99-r3
   7    0.99-r4
   7    0.99-r5
   7    0.99-r6
   7    0.99-r7
   7    0.99-r8
   7    0.99-r9
   8    0.08 r10
   8    0.09 r10
   8    0.09 r11
   8    0.09 r12
   8    0.09 r13
   8    0.10 r10
   8    0.10 r11
   8    0.10 r12
   8    0.10 r13
   8    0.10 r14
   8    0.10 r15
   8    0.10 r16
   8    0.10 r17
   8    0.10 r18
  10    4.1.0.2980
  10    4.1.0.3001
```

So it's 10 characters. (Sorry for the long Bash one-liner)

Also I visited the older code from WinPcap, and it seems it used a
64-byte long string:
[packetNtx/Dll/Packet32.c#L105](https://github.com/wireshark/winpcap/blob/267327e/packetNtx/Dll/Packet32.c#L105)
```
char PacketLibraryVersion[64]; 
```

So I assumed it's safe to allocate the same on stack.
This commit is contained in:
Saulius Krasuckas 2023-12-18 13:00:59 +02:00 committed by GitHub
parent d794b516e1
commit f221984d89
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -79,7 +79,7 @@ int CDECL main(int argc, char **argv)
LPWSTR wstrName;
LPSTR strName, strDesc;
int nAdapterCount;
PCHAR dllVersion;
char dllVersion[64] = { '\0' };
PCHAR testString;
int nDLLMajorVersion, nDLLMinorVersion;
@ -98,7 +98,7 @@ int CDECL main(int argc, char **argv)
}
// Get DLL Version and Tokenize
dllVersion = PacketGetVersion();
strcpy(dllVersion, PacketGetVersion());
nDLLMajorVersion = -1;
nDLLMinorVersion = -1;
for (testString = strtok(dllVersion, ",. ");