Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1932066 - Cannot build with -Werror=stringop-truncation
Summary: Cannot build with -Werror=stringop-truncation
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: libss7
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ben Beasley
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-23 20:10 UTC by Ben Beasley
Modified: 2021-04-12 13:09 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: Bug
Embargoed:


Attachments (Terms of Use)
Complete compiler diagnostic output (9.15 KB, text/plain)
2021-02-23 20:10 UTC, Ben Beasley
no flags Details

Description Ben Beasley 2021-02-23 20:10:05 UTC
Created attachment 1758941 [details]
Complete compiler diagnostic output

Description of problem:

Cannot build with -Werror=stringop-truncation, which is part of the usual hardening flags.

Version-Release number of selected component (if applicable):

All versions

How reproducible:

Steps to Reproduce:
1. Remove the line
     export CFLAGS="${CFLAGS} -Wno-error=stringop-truncation"
   from the spec file.
2. Build the RPM.
3. Observe the RPM build fails with an error.

Actual results:

Cannot build with -Werror=stringop-truncation.

Expected results:

Nothing in the code triggers these warnings.

Additional info:

Currently these warnings are still reported in the build, but are not treated as errors.

These warnings look like real problems, and should be reported upstream.

This bug is to track the problem, and any efforts to resolve it properly.

Comment 1 Fedora Admin user for bugzilla script actions 2021-02-24 02:35:15 UTC
This package has changed maintainer in Fedora. Reassigning to the new maintainer of this component.

Comment 2 Ben Beasley 2021-03-27 13:52:29 UTC
Here is an example of the warnings about strncpy().

> In file included from /usr/include/string.h:519,
>                  from isup.c:32:
> In function 'strncpy',
>     inlined from 'isup_set_calling' at isup.c:2879:4,
>     inlined from 'isup_set_calling' at isup.c:2875:6:
> /usr/include/bits/string_fortified.h:91:10: warning: 'strncpy' specified bound 64 equals destination size [-Wstringop-truncation]
>    91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> void isup_set_calling(struct isup_call *c, const char *calling, unsigned char calling_nai, unsigned char presentation_ind, unsigned char screening_ind)
> {
>         if ((calling && calling[0]) || presentation_ind == SS7_PRESENTATION_ADDR_NOT_AVAILABLE) {
>                 if (calling) {
>                         strncpy(c->calling_party_num, calling, sizeof(c->calling_party_num));
>                 } else {
>                         c->calling_party_num[0] = '\0';
>                 }
>                 c->calling_nai = calling_nai;
>                 c->presentation_ind = presentation_ind;
>                 c->screening_ind = screening_ind;
>         }
> }

There are a number of similar warnings about strncpy() for other fields. In this case, the intent seems to be that the argument “calling” is copied into the struct field with null-termination, truncating any overlong string. But strncpy does not guarantee null-termination. It is likely that the code should have been something like:

>                 if (calling) {
>                         strncpy(c->calling_party_num, calling, sizeof(c->calling_party_num));
>                         c->calling_party_num[sizeof(c->calling_party_num) - 1U] = '\0';
>                 } else {

-----

The other category of strncpy() warnings is like this:

> In function 'strncpy',
>     inlined from 'isup_event_iam' at isup.c:4335:2:
> /usr/include/bits/string_fortified.h:91:10: warning: 'strncpy' output may be truncated copying 50 bytes from a string of length 63 [-Wstringop-truncation]
>    91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That comes from

> strncpy(e->iam.called_party_num, c->called_party_num, sizeof(e->iam.called_party_num));

where e is an ss7_event discriminated union (from libss7.h), with the ss7_event_iam-type member active. That has “char called_party_num[50];”, which is indeed shorter than the called_party_num in “c”, which is a struct isup_call.

What should be done in this case? Is truncation appropriate? It is already happening, only without null termination.

-----

I intend to consult the upstream developers on IRC (Freenode #asterisk-dev) to see what they think of these warnings.

Comment 3 Ben Beasley 2021-04-12 13:09:43 UTC
Upstream issue: https://issues.asterisk.org/jira/browse/SS7-64

Upstream developers have been busy but plan to look at it when they can.


Note You need to log in before you can comment on or make changes to this bug.