[Bug libc/12667] New: fscanf silently converts signed to unsigned

[Bug libc/12667] New: fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

           Summary: fscanf silently converts signed to unsigned
           Product: glibc
           Version: 2.8
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper.fsp@gmail.com
        ReportedBy: alexander.enchevich@yahoo.com


The man page for the scanf functions says:

RETURN VALUE
       These  functions  return the number of input items successfully matched
       and assigned, which can be fewer than provided for, or even zero in the
       event of an early matching failure.


However, the fscanf function (I presume the other scanf funcs will behave the
same), when given a one-line file as input, which looks like:
-666666
will not detect that this is a signed number (see attached file test_fscanf.c,
around lines 84-96).

So a call to fscanf that looks like this:

    unsigned int    nInt = 0;
    ret = fscanf( fptr, " %u", &nInt );

will put will 4294300630 into nInt and will return 1, indicating that 1 field
was successfully scanned and assigned. This is incorrect, because the number
-666666 does not qualify as an "unsigned int". The above call should return
EOF, indicating that the request to scan one *unsigned integer* was NOT
successful, i.e. a "matching failure" occured; and nInt should not be modified. 

The matching failure is also defined in the man page as:
    ... matching failure, meaning that the input was inappropriate (see below).
                               ... If the next item of input does
    not match the conversion specification, the conversion fails —
    this is a matching failure.

This is exactly the case - the next item (the signed int -666666) not matching
the conversion specification (%u) and yet the call does not return EOF, as the
man page says it should on matching failure and does not even return 0 to
indicate it was unable to successfully scan any fields. It simply reads the
number as a *signed int* and then stores the result into the provided *unsigned
int* which explains the value 4294300630 in the example.

Interestingly, at the same time errno is set to 34 (Numerical result outside of
range) so some code inside the lib does have some idea that something went
wrong. This is yet another proof that this is a bug - you can't have errno set
if everything is had supposedly worked fine.

Also, attempting to scan a number bigger than MAXINT (signed and unsigned),
e.g. 6666666666 silently returns 0xFFFFFFFF as the scanned integer, which is
also a bug. (I can file a separate report for it if it is preferable?)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

--- Comment #1 from Alexander Enchevich <alexander.enchevich at yahoo dot com> 2011-04-12 13:25:28 UTC ---
Created attachment 5664
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5664
example src file to illustrate error

gcc -g -o test_fscanf test_fscanf.c

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

Alexander Enchevich <alexander.enchevich at yahoo dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5664|application/octet-stream    |text/plain
          mime type|                            |

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

--- Comment #2 from Alexander Enchevich <alexander.enchevich at yahoo dot com> 2011-04-12 13:29:28 UTC ---
Created attachment 5665
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5665
input data (text)

A text file to give to the example app as input..

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

--- Comment #3 from Alexander Enchevich <alexander.enchevich at yahoo dot com> 2011-04-12 13:37:12 UTC ---
The comment about errno actually relates to the problem with big numbers
(bigger than MAXINT), so the last 2 paragraphs should be switched:

> 
> Also, attempting to scan a number bigger than MAXINT (signed and unsigned),
> e.g. 6666666666 silently returns 0xFFFFFFFF as the scanned integer, which is
> also a bug. (I can file a separate report for it if it is preferable?)
>
> Interestingly, at the same time errno is set to 34 (Numerical result outside of
> range) so some code inside the lib does have some idea that something went
> wrong. This is yet another proof that this is a bug - you can't have errno set
> if everything is had supposedly worked fine.

Sorry about this..

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[schwab@linux-m68k.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID

--- Comment #4 from Andreas Schwab <schwab@linux-m68k.org> 2011-04-12 13:39:12 UTC ---
fscanf:
u  Matches an optionally signed decimal integer, whose format is the same as
expected for the subject sequence of the strtoul function with the value 10
for the base argument. The corresponding argument shall be a pointer to
unsigned integer.

strtoul:
If the subject sequence begins with a minus sign, the value resulting from
the conversion is negated (in the return type).

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

Alexander Enchevich <alexander.enchevich at yahoo dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |

--- Comment #5 from Alexander Enchevich <alexander.enchevich at yahoo dot com> 2011-04-12 16:13:12 UTC ---
I disagree with your decision to close this issue and insist that it is a bug.
Here's why (in ascending order of importance):

1. If you look at this page from the man-pages project:
http://www.kernel.org/doc/man-pages/online/pages/man3/fscanf.3.html , you will
see that the above info reads:
u      Matches  an unsigned decimal integer; the next pointer must be a
       pointer to unsigned int.

So there isn't any references to strtoul here. And there shouldn't be, as
strtoul is not related to the scanf family of functions and this would
introduce an unnecessary dependency on another totally unrelated party
(stroul).

2. Assuming you mean mathematical negation, the result in the above scan
operation is *not* negated.

It would have been negated if the fscanf took the -666666 and returned 666666,
but it doesn't. It simply stores the binary representation of -666666
(FFF5D3D6) into the provided pointer to unsigned int. As-is. Totally useless.
(not that negating it and storing +666666 would have been more useful..)

Looking into the strtoul manpages page the excerpt you mentioned reads:

     The  strtoul() function returns either the result of the conversion or, if
here was a leading minus sign, the negation of the result of the conversion...

Meaning (as I understand it):
  1. Convert provided string to number.
  2. If there was a leading minus sign then negate the number (that is multiply
it by -1.


3. I think you will agree with me that way more important than citing man-pages
and turning this into a "legal battle" is the context of how these functions
are used. 

And the context behind almost every ?scanf call is: we are processing some
input which we *hope* will mostly follow some predefined rules but we know
there *will be cases* when it doesn't. What do we do then? How do we detect the
erroneous conditions? With the current implementation, which happily gives us
back 4294300630 instead of providing some indication that a number beginning
with '-' is obviously not an unsigned number; there's nothing we can do to
process input correctly and safely with these functions. We are basically
forced to implement our own scanf's, reading files char by char, reading always
big signed int's and then filtering out the invalid numbers or some other
jumps-through-hoops along these lines

Please re-consider your decision to dismiss this bug report. Obviously there is
no way to predict what an input stream might produce and and these are
functions intended to process exactly these types of input streams. When we ask
for an unsigned integer and the stream produces a signed one there's got to be
a way to detect this condition. We can't have a function designed to process
live user input not being able to distinguish between -666666 and +4294300630
and giving back the same result for both cases...

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[schwab@linux-m68k.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #6 from Andreas Schwab <schwab@linux-m68k.org> 2011-04-12 16:18:11 UTC ---
The manpage is not relevant, only the C standard is.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[alexander.enchevich at yahoo dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12667

--- Comment #7 from Alexander Enchevich <alexander.enchevich at yahoo dot com> 2011-04-13 06:59:44 UTC ---
You are correct in dismissing my bug report.

After reading the standard docs
(http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf) I can see that the
two excerpts are exactly as you quoted. I understand that this behaviour can
not be considered a bug under the current C standard; that the problem (if we
agree that this is a problem) is not of the implementation but of the design as
defined by the current C Standard; and I understand why you closed the bug
report and agree that it was the right thing to do. 

My apologies - I thought I can rely on the man-pages but I guess I'll have to
revisit this assumption.. :)


cheers..
Alexander
--------
P.S. Andreas, just as a side note: I was wondering what is your personal
opinion on this matter and whether you agree that there is a flaw in the
current design of the scanf/strtoul functions?

In my view the current design does not allow programmers to reliably use the %u
type sequence to scan for unsigned numbers and forces them to use other means
of processing streams where input is expected but *not guaranteed* to follow
some predetermined structure. Therefore, IMHO, the designed behaviour of these
functions as defined by the current C standard, is flawed. (Whether it is a
good idea to "fix" this, given all the stakeholders, is a different story..)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/12667] fscanf silently converts signed to unsigned

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12667

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.