Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

[Lukas' Home Page]

Good morning,

I find out a strange and bad beaviour in functions from scanf() family when
reading one-byte int variable in MinGW, Cygwin and Borland/Embarcadero C
environments (on Windows, on Linux this doesn't happen). This bug is
corelated with MSVCRT library which isn't written in C99 standard (it's
written in C89 i think).

So, the point is, when you're reading one byte using scanf() function, AND
you are using %hhu format specifier, you have Integer Overflow bug in your
code, because MinGW links to old MSVCRT (C89 version) even if you compile
with -std=c99 parameter.

This works, because scanf() in old MSVCRT library doesn't know "h" format
specifier. BUT! The problem is, that scanf try to interpret format specifier
anyway, omits unsupported "h" specifier and it's loading full integer ("u")
to memory (it should omit not supported part of format - whole "%hhu" format
part, not just only "h"). The C99 specification says on 361 page: "If a
conversion specification is invalid, the behavior is undefined." - but it is
WRONG, because the behaviour SHOULD BE DEFINED AS OMITING THE WHOLE
UNSUPPORTED PART OF FORMAT (not only single specifier, but whole part).
In exploit (below), compiler doesn't even display warnings (event if you
compile program with -std=c99 and -Wextra parameters). I compile on Windows
7 using that command:

gcc main.c -o main.exe -Wextra


Exploit example:
===============================

#include <stdio.h>
#include <stdbool.h>

typedef volatile unsigned char uint8_t;

int main()
{
    bool allowAccess = false; // allowAccess should be always FALSE
    uint8_t userNumber;
    char format[] = "%hhu";
    char buffer[] = "257\n";
    sscanf(buffer, format, &userNumber);
    if (allowAccess)
    {
        printf("Access granted: secret information - Lech Walesa was a Bolek
agent\n");
    }
    printf("Entered number is: %d\n", userNumber);
    return 0;
}

Best regards,

---
Łukasz "Lukas" Wyporek
lukas.home.page@gmail.com 
http://www.lukashp.pl 



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

[Brian Inglis]

On 2017-03-05 12:48, Lukas' Home Page wrote:
> I find out a strange and bad beaviour in functions from scanf() family when
> reading one-byte int variable in MinGW, Cygwin and Borland/Embarcadero C
> environments (on Windows, on Linux this doesn't happen). This bug is
> corelated with MSVCRT library which isn't written in C99 standard (it's
> written in C89 i think).
> So, the point is, when you're reading one byte using scanf() function, AND
> you are using %hhu format specifier, you have Integer Overflow bug in your
> code, because MinGW links to old MSVCRT (C89 version) even if you compile
> with -std=c99 parameter.
> This works, because scanf() in old MSVCRT library doesn't know "h" format
> specifier. BUT! The problem is, that scanf try to interpret format specifier
> anyway, omits unsupported "h" specifier and it's loading full integer ("u")
> to memory (it should omit not supported part of format - whole "%hhu" format
> part, not just only "h"). The C99 specification says on 361 page: "If a
> conversion specification is invalid, the behavior is undefined." - but it is
> WRONG, because the behaviour SHOULD BE DEFINED AS OMITING THE WHOLE
> UNSUPPORTED PART OF FORMAT (not only single specifier, but whole part).

Dealing safely with anything other than int, double, and width limited strings 
using scanf requires a lot of care, by the implementation and the developer.
Undefined behaviour puts the onus on you as a developer to avoid invoking 
whatever behaviour the implementation defaults to.
I can suggest looking into the inttypes.h SCNu* macros and stdint.h.
Options -Wall and -Wextra should produce format warnings.
Cygwin uses newlib and Mingw may use its own scanf library.
Cygwin provides a Mingw port, so any problems should be reported upstream.

> In exploit (below), compiler doesn't even display warnings (event if you
> compile program with -std=c99 and -Wextra parameters). I compile on Windows
> 7 using that command:
> gcc main.c -o main.exe -Wextra
> #include <stdio.h>
> #include <stdbool.h>
> typedef volatile unsigned char uint8_t;
> int main()
> {
>     bool allowAccess = false; // allowAccess should be always FALSE
>     uint8_t userNumber;
>     char format[] = "%hhu";
>     char buffer[] = "257\n";
>     sscanf(buffer, format, &userNumber);
>     if (allowAccess)
>     {
>         printf("Access granted: secret information - Lech Walesa was a Bolek
> agent\n");
>     }
>     printf("Entered number is: %d\n", userNumber);
>     return 0;
> }

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

[Brian Inglis]

On 2017-03-05 15:50, Brian Inglis wrote:
> On 2017-03-05 12:48, Lukas' Home Page wrote:
>> I find out a strange and bad beaviour in functions from scanf() family when
>> reading one-byte int variable in MinGW, Cygwin and Borland/Embarcadero C
>> environments (on Windows, on Linux this doesn't happen). This bug is
>> corelated with MSVCRT library which isn't written in C99 standard (it's
>> written in C89 i think).
>> So, the point is, when you're reading one byte using scanf() function, AND
>> you are using %hhu format specifier, you have Integer Overflow bug in your
>> code, because MinGW links to old MSVCRT (C89 version) even if you compile
>> with -std=c99 parameter.
>> This works, because scanf() in old MSVCRT library doesn't know "h" format
>> specifier. BUT! The problem is, that scanf try to interpret format specifier
>> anyway, omits unsupported "h" specifier and it's loading full integer ("u")
>> to memory (it should omit not supported part of format - whole "%hhu" format
>> part, not just only "h"). The C99 specification says on 361 page: "If a
>> conversion specification is invalid, the behavior is undefined." - but it is
>> WRONG, because the behaviour SHOULD BE DEFINED AS OMITING THE WHOLE
>> UNSUPPORTED PART OF FORMAT (not only single specifier, but whole part).
> 
> Dealing safely with anything other than int, double, and width
> limited strings using scanf requires a lot of care, by the
> implementation and the developer.
> Undefined behaviour puts the onus on you as a developer to avoid
> invoking whatever behaviour the implementation defaults to.
> I can suggest looking into the inttypes.h SCNu* macros and stdint.h.
> Options -Wall and -Wextra should produce format warnings.
> Cygwin uses newlib and Mingw may use its own scanf library.
> Cygwin provides a Mingw port, so any problems should be reported
> upstream.
>> In exploit (below), compiler doesn't even display warnings (event
>> if you compile program with -std=c99 and -Wextra parameters). I
>> compile on Windows 7 using that command:
>> gcc main.c -o main.exe -Wextra
>> #include <stdio.h>
>> #include <stdbool.h>
>> typedef volatile unsigned char uint8_t;
>> int main()
>> {
>>     bool allowAccess = false; // allowAccess should be always FALSE
>>     uint8_t userNumber;
>>     char format[] = "%hhu";
>>     char buffer[] = "257\n";
>>     sscanf(buffer, format, &userNumber);
>>     if (allowAccess)
>>     {
>>         printf("Access granted: secret information - Lech Walesa was a Bolek agent\n");
>>     }
>>     printf("Entered number is: %d\n", userNumber);
>>     return 0;
>> }

 
Compiling this in Cygwin 64 gives:

error: conflicting type qualifiers for ‘uint8_t’
 typedef volatile unsigned char uint8_t;
                                ^
...
/usr/include/sys/_stdint.h:24:19: note: previous declaration of ‘uint8_t’ was here
 typedef __uint8_t uint8_t ;
                   ^
as *_t is reserved by POSIX and libc.

Changing this to a regular identifer compiles cleanly and running gives:

Entered number is: 1

as might be expected as a result of the equivalent function call:

	userNumber = strtoul(buffer,NULL,10);

Unsure what effect volatile is intended to provide in this instance, 
other than inhibiting any possible optimization to uses of userNumber?
Cygwin gcc does not optimize out the function calls to sscanf and printf,
although it does convert the conditional printf to a puts at low levels 
and eliminates it at high levels.

So if Mingw gcc/sscanf or Borland/Embarcadero cc are not behaving 
correctly, please report those problems to the appropriate upstream 
mailing lists.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

[Thomas Wolff]

Am 05.03.2017 um 20:48 schrieb Lukas' Home Page:
> Good morning,
>
> I find out a strange and bad beaviour in functions from scanf() family when
> reading one-byte int variable in MinGW, Cygwin and Borland/Embarcadero C
> environments (on Windows, on Linux this doesn't happen). This bug is
> corelated with MSVCRT library which isn't written in C99 standard (it's
> written in C89 i think).
>
> So, the point is, when you're reading one byte using scanf() function, AND
> you are using %hhu format specifier, you have Integer Overflow bug in your
> code, because MinGW links to old MSVCRT (C89 version) even if you compile
> with -std=c99 parameter.
>
> This works, because scanf() in old MSVCRT library doesn't know "h" format
> specifier. BUT! The problem is, that scanf try to interpret format specifier
> anyway, omits unsupported "h" specifier and it's loading full integer ("u")
> to memory (it should omit not supported part of format - whole "%hhu" format
> part, not just only "h"). The C99 specification says on 361 page: "If a
> conversion specification is invalid, the behavior is undefined." - but it is
> WRONG, because the behaviour SHOULD BE DEFINED AS OMITING THE WHOLE
> UNSUPPORTED PART OF FORMAT (not only single specifier, but whole part).
> In exploit (below), compiler doesn't even display warnings (event if you
> compile program with -std=c99 and -Wextra parameters). I compile on Windows
> 7 using that command:
>
> gcc main.c -o main.exe -Wextra
>
>
> Exploit example:
> ===============================
>
> #include <stdio.h>
> #include <stdbool.h>
>
> typedef volatile unsigned char uint8_t;
>
> int main()
> {
>      bool allowAccess = false; // allowAccess should be always FALSE
>      uint8_t userNumber;
>      char format[] = "%hhu";
>      char buffer[] = "257\n";
>      sscanf(buffer, format, &userNumber);
>      if (allowAccess)
>      {
>          printf("Access granted: secret information - Lech Walesa was a Bolek
> agent\n");
>      }
>      printf("Entered number is: %d\n", userNumber);
>      return 0;
> }
What output do you get (and on which of the mentioned systems),
what output do you expect (and why),
and how is Lech Walesa involved in the issue?
For me, the program produces correct output on cygwin, according to
cygwin's man page of sscanf, and I don't care which library is being used.
Thomas

---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)

[Hans-Bernhard Bröker]

Am 05.03.2017 um 20:48 schrieb Lukas' Home Page:
> Good morning,
>
> I find out a strange and bad beaviour in functions from scanf() family when
> reading one-byte int variable in MinGW, Cygwin and Borland/Embarcadero C
> environments

Actually, I'm pretty sure this only happens on MinGW.  Neither the 
problem nor your explanation applies to Cygwin, and I find it very hard 
to believe that your explanation could apply to Borland/Embarcadero C.


> This bug is
> corelated with MSVCRT library which isn't written in C99 standard (it's
> written in C89 i think).

Close, but not quite correct.  The bug is in MinGW, and the nature of 
the bug is that they still use MSVCRT, even if a C99 build was asked 
for.  As long as MingW puts uses the msvcrt.dll versions of the *scanf() 
functions (instead of one of its later replacements, e.g. mscvr120.dll, 
or rolling their own), they simply cannot implement C99.

But that's not for Cygwin, but rather for MinGW to address.  So please 
take this issue to them.

> This works, because scanf() in old MSVCRT library doesn't know "h" format
> specifier.

That claim is wrong on two counts.  That "h" is not a format specifier, 
but rather a length modifier here.  And MSVCRT does know "h".  What it 
doesn't recognize (because of its age, and Microsoft's long-time refusal 
to act on the C99 standard) is the doubled-up "hh" modifier.

> The C99 specification says on 361 page:

To a considerable extent, what the C99 specification says on this matter 
is irrelevant, because MSVCRT never claimed to implement C99. That 
doesn't change this argument though, because the same wording was in 
C90, too:

> "If a
> conversion specification is invalid, the behavior is undefined." - but it is
> WRONG, because the behaviour SHOULD BE DEFINED AS OMITING THE WHOLE
> UNSUPPORTED PART OF FORMAT

It's not nearly as wrong as to justify you SCREAMING about it.  Nor is 
it even up to you to decide what is wrong in an international standard 
document.  Just because you would prefer different wording doesn't make 
it wrong.

> #include <stdio.h>
> #include <stdbool.h>
>
> typedef volatile unsigned char uint8_t;
>
> int main()
> {
>     bool allowAccess = false; // allowAccess should be always FALSE
>     uint8_t userNumber;
>     char format[] = "%hhu";
>     char buffer[] = "257\n";
>     sscanf(buffer, format, &userNumber);

This code causes undefined behaviour even in a fully correct C99 
implementation, because it tries to convert the value 257 into an 8-bit 
object.  (C99 7.19.6.2p10, last sentence).  That renders it unfit to 
demonstrate anything.



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple