avahi-core: fix -Wformat-truncation in entry.c and iface.c#900
Conversation
|
Thank you for the PR.
I don't think it's accurate. For example avahi actually silently truncates and advertises the truncated Other than that I think this PR should be split into several PRs where every warning is addressed separately. The warnings themselves and the environment where they are triggered should go to the commit messages as well. |
Since the other commits affect NDEBUG builds I think it would make sense to set up NDEBUG CI jobs and make them green first. As far as I can see with the master branch the tests currently fail under ASan with FAIL: strlst-test
=================
AddressSanitizer:DEADLYSIGNAL
=================================================================
==25435==ERROR: AddressSanitizer: stack-overflow on address 0x7fffb4b96000 (pc 0x558cc9718209 bp 0x7fffb4b93b90 sp 0x7fffb4b938c0 T0)
#0 0x558cc9718209 in avahi_string_list_to_string /home/runner/work/avahi/avahi/avahi-common/strlst.c:173:21
#1 0x558cc971e155 in main /home/runner/work/avahi/avahi/avahi-common/strlst-test.c:85:9
#2 0x7f7fd882a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#3 0x7f7fd882a28a in __libc_start_main csu/../csu/libc-start.c:360:3
#4 0x558cc963e344 in _start (/home/runner/work/avahi/avahi/avahi-common/strlst-test+0x2e344) (BuildId: ad1c456aeeea3678d3125953b3a2d9299ed8e42b)and the compiler actually complains about the place responsible for that strlst-test.c: In function 'main':
strlst-test.c:37:9: warning: variable 'r' set but not used [-Wunused-but-set-variable]
37 | int r;
| ^
strlst-test.c:83:5: warning: 'b' is used uninitialized [-Wuninitialized]
83 | printf("equal: %i\n", avahi_string_list_equal(a, b));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
strlst-test.c:35:33: note: 'b' was declared here
35 | AvahiStringList *a = NULL, *b, *p;
| ^
mv -f .deps/strlst_test-strlst-test.Tpo .deps/strlst_test-strlst-test.Po(I'd also add that it isn't recommended to build avahi with NDEBUG in general but the places where stuff with side effects is placed in assertions should be fixed of course. Either way it should be done carefully with actual tests making sure that it isn't completely broken). |
|
NDEBUG builds are now tested under ASan/UBSan and Valgrind by the CI: #901 |
Split into separate PRs. This one kept a# the truncate PR |
As mentioned it isn't correct. |
GCC cannot see through Avahi's domain name validation logic and warns
that snprintf output may be truncated. The buffers are correctly sized
for validated input; suppress the false positive warnings with pragmas.
fixes:
entry.c: In function 'server_add_dns_server_name':
entry.c:937:32: warning: '%s' directive output may be truncated writing up to 1013 bytes into a region of size between 997 and 1001 [-Wformat-truncation=]
937 | snprintf(t, sizeof(t), "%s.%s", type == AVAHI_DNS_SERVER_RESOLVE ? "_domain._udp" : "_dns-update._udp", normalized_d);
| ^~ ~~~~~~~~~~~~
entry.c:937:5: note: 'snprintf' output between 14 and 1031 bytes into a destination of size 1014
937 | snprintf(t, sizeof(t), "%s.%s", type == AVAHI_DNS_SERVER_RESOLVE ? "_domain._udp" : "_dns-update._udp", normalized_d);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
entry.c: In function 'avahi_server_add_dns_server_address':
entry.c:994:37: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size 60 [-Wformat-truncation=]
994 | snprintf(n, sizeof(n), "ip6-%s.%s", h, domain);
| ^~ ~
entry.c:994:9: note: 'snprintf' output 6 or more bytes (assuming 69) into a destination of size 64
994 | snprintf(n, sizeof(n), "ip6-%s.%s", h, domain);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
entry.c:989:36: warning: '%s' directive output may be truncated writing up to 63 bytes into a region of size 61 [-Wformat-truncation=]
989 | snprintf(n, sizeof(n), "ip-%s.%s", h, domain);
| ^~ ~
entry.c:989:9: note: 'snprintf' output 5 or more bytes (assuming 68) into a destination of size 64
989 | snprintf(n, sizeof(n), "ip-%s.%s", h, domain);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
iface.c: In function 'avahi_hw_interface_update_rrs':
iface.c:143:45: warning: ' [' directive output may be truncated writing 2 bytes into a region of size between 1 and 64 [-Wformat-truncation=]
143 | snprintf(name, sizeof(name), "%s [%s]", unescaped, mac);
| ^~
iface.c:143:13: note: 'snprintf' output between 4 and 322 bytes into a destination of size 64
143 | snprintf(name, sizeof(name), "%s [%s]", unescaped, mac);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Understood, just trying to get them all separated first. In this case - the question could/should be: what is the right size of the these declarations, as the MAXs here should honored - if they are correct, and can’t be resized to a %s.%s due to “mandates maximum length” /** The maximum length of a a fully escaped domain name C string. This
* is calculated like this: RFC1034 mandates maximum length of FQDNs
* is 255. The maximum label length is 63. To minimize the number of
* (non-escaped) dots, we comprise our maximum-length domain name of
* four labels á 63 characters plus three inner dots. Escaping the
* four labels quadruples their length at maximum. An escaped domain
* name has the therefore the maximum length of 63*4*4+3=1011. A
* trailing NUL and perhaps two unnecessary dots leading and trailing
* the string brings us to 1014. */
#define AVAHI_DOMAIN_NAME_MAX 1014
/** Maximum size of an unescaped label */
#define AVAHI_LABEL_MAX 64 |
avahi-core: fix -Wformat-truncation in entry.c and iface.c
GCC warns that snprintf output may be truncated in three call sites. The buffers are correctly sized for validated input and truncation cannot occur in practice; GCC cannot see through Avahi's domain name validation logic. Suppress the false positives with local #pragma GCC diagnostic guards rather than resizing buffers, which would misrepresent the actual data constraints.