Why does strlen() not give the right length for these command line arguments when they are null-terminated?

Question

I searched a lot on SO, but only find these type of questions:

• How to find the length of argv[] in C
• strlen sometimes equal to sizeof for null-terminated strings
• Searching for strings that are NULL terminated within a file where they are not NULL terminated
• strlen not giving correct string length C

Consider the following code, where a bunch of variables are declared and initialized:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <unistd.h>
#include <string.h>
int main(int argc, char **argv)
{
    char *src_ip_str, *dst_ip_str, *payload, *dp_str;
    u_int32_t src_ip = 0, dst_ip = 0;
    u_int16_t dp;
    if(argc != 5){
        //exit
    }  
    int l_src = strlen(argv[1]);
    int l_dst = strlen(argv[2]);
    int l_dp  = strlen(argv[3]);
    int l_pay = strlen(argv[4]);

    src_ip_str  = (char *)malloc(l_src * sizeof(char)+1);
    dst_ip_str  = (char *)malloc(l_dst * sizeof(char)+1);
    dp_str      = (char *)malloc(l_dp  * sizeof(char)+1);
    payload     = (char *)malloc(l_pay * sizeof(char)+1);

    strncpy(src_ip_str, argv[1], l_src); src_ip_str[l_src] = '\0';
    strncpy(dst_ip_str, argv[2], l_dst); src_ip_str[l_dst] = '\0';
    strncpy(dp_str    , argv[3], l_dp ); src_ip_str[l_dp ] = '\0';
    strncpy(payload   , argv[4], l_pay); //don't know if this should be null-terminated since it's TCP payload and don't know if it truncated the null-terminated string.
    dp = atoi(dp_str);
    // This is where I end GDB.

Some code was ommited, but this code clearly is acting strange. The program is invoked as such: sudo ./program <ip_src> <ip_dst> <port_dst> <payload_string>

Example: sudo ./program 1.1.19.70 172.16.1.255 513 some_payload

There is one bug and one potential bug:

1. The src_ip_str gets truncated to 3 characters when dp_str is initialized with malloc. Before the initialization of dp_str it stays the full IP that is given (i.e. 1.1.19.70).
2. l_dst, l_dp, l_pay do not give correct length values when examined in GDB with p .

I don't get this. When I inspect the memory of the arguments in strings and characters I get:

Strings:

(gdb) p argv[1]
$1 = 0xbffff86a "1.1.19.70"
(gdb) p argv[2]
$2 = 0xbffff875 "172.16.1.255"
(gdb) p argv[3]
$3 = 0xbffff882 "513"
(gdb) p argv[4]
$4 = 0xbffff886 "some_payload"

Characters (edit: I'm noticing that the characters come from an earlier debug session hence some values are off):

0xbffff86a:     49 '1'  57 '9'  46 '.'  48 '0'  46 '.'  49 '1'  57 '9'  46 '.'
0xbffff872:     56 '8'  57 '9'  0 '\000'        49 '1'  55 '7'  50 '2'  46 '.'  49 '1'
0xbffff87a:     54 '6'  46 '.'  49 '1'  46 '.'  50 '2'  53 '5'  53 '5'  0 '\000'
0xbffff882:     53 '5'  49 '1'  51 '3'  0 '\000'        115 's' 111 'o' 109 'm' 101 'e'
0xbffff88a:     95 '_'  112 'p' 97 'a'  121 'y' 108 'l' 111 'o' 97 'a'  100 'd'

Clearly these strings are null-terminated.

I really don't get why strlen(param) gives such strange values. Even when I cast it to size_t I get ridiculous values, e.g.:

(gdb) p (size_t)l_src
$12 = 10
(gdb) p (size_t)l_dst
$13 = 3086757876
(gdb) p (size_t)l_dp
$14 = 134516496
(gdb) p (size_t)l_pay
$15 = 3221223000

Does anyone have an idea what could be happening?

Compiler settings are:

gcc -ggdb -Wall `libnet-config --defines` `libnet-config --libs` program.c -o program

GDB settings are:

sudo gdb --args ./program 1.1.19.70 172.16.1.255 513 some_payload

Answer 1

In this code:

strncpy(src_ip_str, argv[1], l_src); src_ip_str[l_src] = '\0';
strncpy(dst_ip_str, argv[2], l_dst); src_ip_str[l_dst] = '\0';
strncpy(dp_str    , argv[3], l_dp ); src_ip_str[l_dp ] = '\0';

you never null-terminated dst_ip_str and dp_str.

This kind of bug is why many people consider strncpy a less-safe version of strcpy, and we avoid it at all costs. This part of your code could have been written in a simpler and less error-prone fashion:

strcpy(src_ip_str, argv[1]);
strcpy(dst_ip_str, argv[2]);
strcpy(dp_str,     argv[3]);

You may as well strcpy payload as well, since it is coming into you as a null-terminated string.

You may use the non-ISO C function strdup to replace the combination of malloc and strcpy. (This is only guaranteed to exist on POSIX, but if porting to a system that doesn't have it, it's trivial to roll your own).

In either case it would be a good idea to check for allocation failure if you are interested in writing robust code.

Answer 2

Credits go to +Hans Passant, his name should be under this answer (don't know how to make it happen, if he posts an answer I'll delete this one). It's a classic version of looking long at one's code while the answer is staring you in the face.

The error is here:

strncpy(src_ip_str, argv[1], l_src); src_ip_str[l_src] = '\0';
strncpy(dst_ip_str, argv[2], l_dst); src_ip_str[l_dst] = '\0';
strncpy(dp_str    , argv[3], l_dp ); src_ip_str[l_dp ] = '\0';

I null-terminated src_ip_str[x] three times! I feel shame, nevertheless I do feel compelled to answer what happens exactly.

Consider the source IP: 1.1.19.70 (GNU standard time ;) ) and destination IP: 172.16.1.255.

1. The first time you have: 1.1.19.70\0
2. The second time you have: 1.1.19.70\0x\0 (the destination IP is longer)
3. The third time you have: 1.1\019.70\0x\0

I should use strdup() or write less code to make less errors with null-terminated strings.

Thank you all for helping! :)