Bug 1431678 - gcc7, frivolous warning messages for snprintf
Summary: gcc7, frivolous warning messages for snprintf
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 26
Hardware: x86_64
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-03-13 15:23 UTC by Leslie Satenstein
Modified: 2017-04-23 01:58 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-04-21 07:57:39 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Leslie Satenstein 2017-03-13 15:23:39 UTC
Description of problem:
GCC7 tells me that snprintf can overflow.

 ./src/desnew.c:449:17: note: ‘snprintf’ output between 9 and 21 bytes into a destination of size 8
                 snprintf(desparms.iv,sizeof(desparms.iv), "%08ld", d3metaio.jdate);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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

gcc version 7.0.1 20170309 (Red Hat 7.0.1-0.12) (GCC) 

How reproducible:

when input is potentially greater than the target field.

Steps to Reproduce:
1.
2.
3.

Actual results:



Expected results:

I would expect no warning message for snprintf(...), buf would expect the warning for   
sprintf( ...)

The 2nd parameter of snprintf() is protecting the overrun of the output field via it's value, typically the "sizeof(parameter1) -1" 

Additional info:

From my makefile

CC:=gcc
CFLAGS = -O3 -Wall

Comment 1 Florian Weimer 2017-03-13 15:31:02 UTC
The intent of this warning is to capture unexpected snprintf results due to truncation.  Is this warning bogus?  We need more context to tell if this is the case.

(You probably shouldn't be using DES anymore, and the IV doesn't look very random.)

Comment 2 Leslie Satenstein 2017-04-19 18:09:19 UTC
Florian, Never saw the email reply, sorry for not replying sooner.

Here is info

I have a report and the report has a header record including a field for date and page number and some other adjacent information. 


I know the length of the header date field. and a page number field, and to insure that I do not include the null terminator, I use 

(comment 1 example)


gcc7 warns me that my target is too short to receive the information. I know that and that is why I use snprintf(), to control output length. 
I set snprintf() values to exclude the null character.

I use  gcc -O3 -Wall -Wextra  ... and I have my code compile cleanly so that there are no warning messages. 

I would expect a warning message if I had used sprintf()



Now I am obliged to: 
a) write to a second area and do a memcpy () from that second area to the target. 

or

b) Save the location where the null character would overwrite the first byte of the next adjacent field and then restore after the snprintf()

gcc 6.x does not produce a warning message for snprintf() if the source field equals the target. 

desparms.iv is exactly 8 characters long. (refer to comment 1).

Unrelated to this bug...
3Des is used internally for inter-department transfers, AES for outgoing  (external) files.

Comment 3 Martin Sebor 2017-04-19 20:10:32 UTC
"%08ld" results in 8 or more digits (possibly preceded by the minus sign) plus the terminating nul but the second argument to snprintf is 8, resulting in truncation.   Unhandled output truncation is typically a bug in the program.  Here, since the width is explicitly specified to be the same as the size of the buffer, the bug seems clear: there's no room for the final digit.

In cases when truncation is expected the caller typically checks the return value from the function and handles it somehow (e.g., by branching on it).  In those cases the warning is not issued.  The source line printed by the warning suggests that this is not one of those cases.  The warning is doing what it was designed to do.

The following small test case illustrates the problem.  Note the output is all zeros as a result of the truncation.

$ cat a.c && gcc -O2 -Wall a.c && ./a.out 
#include <stdio.h>

char d[8];

long n = 1;

int main (void)
{
  snprintf (d, sizeof d, "%08ld", n);
  printf ("%s\n", d);
  return 0;
}
a.c: In function ‘main’:
a.c:9:32: warning: ‘snprintf’ output truncated before the last format character [-Wformat-truncation=]
   snprintf (d, sizeof d, "%08ld", n);
                                ^
a.c:9:3: note: ‘snprintf’ output between 9 and 21 bytes into a destination of size 8
   snprintf (d, sizeof d, "%08ld", n);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0000000

Comment 4 Leslie Satenstein 2017-04-21 03:54:23 UTC
Please clarify C99 STMT

https://linux.die.net/man/3/snprintf

The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). 

ThenV the above link STATES WHAT HAPPENS IF RECEIVING FIELD IS SHORTER THAN SENDING FIELD It says...


If the output was truncated due to this limit then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated. (See also below under NOTES.) 

YOUR EXAMPLE (with just 2 slight changes, to demonstrate

#include <stdio.h>

char d[]="12345678abc";          //want to see  00000001abc after snprintf()

long n = 1;

int main (void)
{
  snprintf (d, 8, "%8.8ld", n);    //minor change here.
  printf ("%s\n", d);
  return 0;
}

The NULL WOULD Have been in the 9th position.  But I truncated at 8. The NULL IS NOT INCLUDED BUT....

The Standard does not say that snprintf() always includes a terminating null if the receiving field is restricted in size..  

If I am wrong about this,then I will accept Not A BUG.

Comment 5 Jakub Jelinek 2017-04-21 06:02:29 UTC
(In reply to Leslie Satenstein from comment #4)
> char d[]="12345678abc";          //want to see  00000001abc after snprintf()
> 
> long n = 1;
> 
> int main (void)
> {
>   snprintf (d, 8, "%8.8ld", n);    //minor change here.
>   printf ("%s\n", d);
>   return 0;
> }
> 
> The NULL WOULD Have been in the 9th position.  But I truncated at 8. The
> NULL IS NOT INCLUDED BUT....
> 
> The Standard does not say that snprintf() always includes a terminating null
> if the receiving field is restricted in size..  
> 
> If I am wrong about this,then I will accept Not A BUG.

You are wrong about this.  ISO C99 when you are citing it, says in 7.19.6.5:
"If n is zero, nothing is written, and s may be a null pointer. Otherwise, output characters beyond the n-1st are discarded rather than being written to the array, and a null character is written at the end of the characters actually written into the array."

Therefore, you won't see "00000001abc", but "0000000\0abc".

Comment 6 Florian Weimer 2017-04-21 06:46:31 UTC
Note that many non-standard variants of snprintf do *not* guarantee NUL termination, for example:

https://msdn.microsoft.com/en-us/library/2ts7cx93(v=vs.110).aspx

In any case, it looks like the warning caught a real bug.

Comment 7 Marek Polacek 2017-04-21 07:57:39 UTC
Closing, given Comment 5.

Comment 8 Leslie Satenstein 2017-04-23 01:58:42 UTC
Florian, 

Thanks for the response and the proof. This response is an appreciation of your patience.  

I patched my code to eliminate to eliminate that 1 byte null.


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