skupperproject / skupper-router

An application-layer router for Skupper networks
https://skupper.io
Apache License 2.0
14 stars 18 forks source link

src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread] #1017

Open jiridanek opened 1 year ago

jiridanek commented 1 year ago
In function 'vflow_set_string',
    inlined from 'vflow_set_string' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1385:6,
    inlined from '_send_response_line' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2009:5:
/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^

https://github.com/skupperproject/skupper-router/actions/runs/4511575739/jobs/7943886750#step:23:423

/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^
In function 'vflow_set_string',
    inlined from 'vflow_set_string' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1385:6,
    inlined from '_send_response_line' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2009:5:
/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^
In function 'vflow_set_string',
    inlined from 'vflow_set_string' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1385:6,
    inlined from '_send_response_line' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2009:5:
/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^
In function 'vflow_set_string',
    inlined from 'vflow_set_string' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1385:6,
    inlined from '_send_response_line' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2009:5:
/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^
In function 'vflow_set_string',
    inlined from 'vflow_set_string' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1385:6,
    inlined from '_send_response_line' at /home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2009:5:
/home/runner/work/skupper-router/skupper-router/skupper-router/src/vanflow.c:1393:44: warning: 'strnlen' specified bound 300 exceeds source size 32 [-Wstringop-overread]
 1393 |         work->value.string_val = !!value ? strndup(value, strnlen(value, MAX_STRING_VALUE)) : 0;
      |                                            ^
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c: In function '_send_response_line':
/home/runner/work/skupper-router/skupper-router/skupper-router/src/adaptors/http1/http1_client.c:2007:10: note: source object allocated here
 2007 |     char code_str[32];
      |          ^

https://github.com/skupperproject/skupper-router/actions/runs/4511575739/jobs/7943886750#step:23:448

kgiusti commented 1 year ago

I'm seeing these warnings also, but they appear to be false-positives. In every case the input string is null terminated and won't exceed the max passed in. What's funny is that the compiler is "smart" enough to determine the array size used by the caller, but not smart enough to determine that the code properly null-terminates the input string and avoid the warning.

Or just as likely I'm not smart enough to see the actual error path the compiler is concerned about.

Should the vanflow code be changed to use plain old strlen() and assert if the return value is > MAX_STRING_VALUE? Or disable the warning flag?

@ted-ross your call.

kgiusti commented 1 year ago

I'm seeing these warnings also, but they appear to be false-positives. In every case the input string is null terminated and won't exceed the max passed in. What's funny is that the compiler is "smart" enough to determine the array size used by the caller, but not smart enough to determine that the code properly null-terminates the input string and avoid the warning.

Or just as likely I'm not smart enough to see the actual error path the compiler is concerned about.

Should the vanflow code be changed to use plain old strlen() and assert if the return value is > MAX_STRING_VALUE? Or disable the warning flag?

@ted-ross your call.

jiridanek commented 1 year ago

I think I sort of get the point. If you inline the calls in one of the places that emit the warning, you get

    #define MAX_STRING_VALUE 300

    char code_str[32];
    snprintf(code_str, 32, "%d", status_code);
    struct vflow_work_t *work = ...;
// [...]
    work->value.string_val    = !!code_str ? strndup(code_str, strnlen(code_str, MAX_STRING_VALUE)) : 0;

So, the compiler wonders: "I know code_str won't be longer than 32 characters, so why I am getting that 300 as a length bound? Lets throw a warning."