DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:subject:references:to:reply-to:from:message-id
	:date:mime-version:in-reply-to:content-type
	:content-transfer-encoding; q=dns; s=default; b=V/t42iwm77hrW5Ep
	LzEL7iiiTk5hJkTNN3RQbNLvd4Z27Ho6wYiPsqJRuG+WY8xU6ldRj2agcepamzpq
	mwVjXT/CdA3Oe6HxHnyCCNOQUjCPzsO5U6+oNSGvdQ+dR4SMT3UsGCazU29t8not
	p8iudYWMGx8mCXgCWEUXHW0AhRw=
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Subject: Re: Integer overflow in functions from scanf() family in MinGW, Cygwin, Borland/Embarcadero C environments (Windows)
References: <001101d295e9$65841800$308c4800$@gmail.com> <fbb14e7b-12e0-5ee3-5066-69c69f8c2643 AT SystematicSw DOT ab DOT ca>
To: cygwin AT cygwin DOT com
Reply-To: Brian DOT Inglis AT SystematicSw DOT ab DOT ca
From: Brian Inglis <Brian DOT Inglis AT SystematicSw DOT ab DOT ca>
Message-ID: <f96a131f-f22b-f73e-88a0-1cec88fa97c4@SystematicSw.ab.ca>
Date: Sun, 5 Mar 2017 22:37:36 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1
MIME-Version: 1.0
In-Reply-To: <fbb14e7b-12e0-5ee3-5066-69c69f8c2643@SystematicSw.ab.ca>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

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