Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing compile-time warnings when compiling with OpenOSC #6

Open
ariel-miculas opened this issue Nov 20, 2023 · 1 comment
Open

Missing compile-time warnings when compiling with OpenOSC #6

ariel-miculas opened this issue Nov 20, 2023 · 1 comment

Comments

@ariel-miculas
Copy link

Example:
fuzzer.c:

#include <stdio.h>
#include <string.h>
#include<stdlib.h>
int main() {
	char src[] = "lorem ipsum dolor sit amet";

	char *dst;
	dst = malloc(10);

	strcpy(dst, src);
	printf("%s\n", dst);
}

Makefile:

CC=gcc
CFLAGS=-I. -Wall -Wextra -Werror -g -O2
LDFLAGS=-lopenosc
OBJ = fuzzer.o

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

fuzzer: $(OBJ)
	$(CC) -o $@ $^ $(CFLAGS) $(LDFLAGS)

.PHONY:clean
clean:
	rm fuzzer *.o

Result:

$ make
gcc -c -o fuzzer.o fuzzer.c -I. -Wall -Wextra -Werror -g -O2 -lopenosc
fuzzer.c: In function ‘main’:
fuzzer.c:10:9: error: ‘strcpy’ writing 27 bytes into a region of size 10 [-Werror=stringop-overflow=]
   10 |         strcpy(dst, src);
      |         ^~~~~~~~~~~~~~~~
fuzzer.c:8:15: note: destination object of size 10 allocated by ‘malloc’
    8 |         dst = malloc(10);
      |               ^~~~~~~~~~
fuzzer.c:10:9: error: ‘strcpy’ forming offset [10, 26] is out of the bounds [0, 10] [-Werror=array-bounds=]
   10 |         strcpy(dst, src);
      |         ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:7: fuzzer.o] Error 1

Now add -include openosc.h to CFLAGS,
Makefile:

CC=gcc
CFLAGS=-I. -Wall -Wextra -Werror -g -O2 -include openosc.h
LDFLAGS=-lopenosc
OBJ = fuzzer.o

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

fuzzer: $(OBJ)
	$(CC) -o $@ $^ $(CFLAGS) $(LDFLAGS)

.PHONY:clean
clean:
	rm fuzzer *.o

And run make again:

$ make
gcc -c -o fuzzer.o fuzzer.c -I. -Wall -Wextra -Werror -g -O2 -include openosc.h -lopenosc
gcc -o fuzzer fuzzer.o -I. -Wall -Wextra -Werror -g -O2 -include openosc.h -lopenosc

The compile time errors have disappeared and have been moved to runtime:

$ ./fuzzer
lorem ips
$ journalctl --since "2 min ago"
Nov 20 22:24:09 archlinux-cisco fuzzer[319384]: DATACORRUPTION-DATAINCONSISTENCY: openosc 1.0.6 Copy error -Traceback= ./fuzzer +0x10bb libc.so.6+0x27cd0 libc.so.6+0x27d8a +0x1115

Using OpenOSC, we've transitioned from a program which didn't compile to an invalid program which truncates the result and reports the buffer overflow at runtime. This is opposite from what is usually desired, i.e. fail as early as possible, catch errors at compile-time rather than at runtime.

@yonhan3
Copy link
Contributor

yonhan3 commented Dec 7, 2023

Thanks for reporting this issue.

There seems some difference between "char src[] = " and "char *src = ". For the first one, somehow, the compiler fails to know the src string length is constant.

If I change the definition of src to "char *src =", then OpenOSC is able to catch the overflow at compile time.

root@2751ad9120bb:/home/openosc-dir/fuzz# diff fuzzer.c fuzzer2.c
5c5
< 	char src[] = "lorem ipsum dolor sit amet";
---
> 	char *src = "lorem ipsum dolor sit amet";
root@2751ad9120bb:/home/openosc-dir/fuzz# gcc -c -o fuzzer2.o fuzzer2.c -I. -g -O2 -I/home/openosc-dir/OpenOSC/include -DOPENOSC_METRIC_FEATURE_ENABLED -U_FORTIFY_SOURCE -include openosc.h
In file included from /home/openosc-dir/OpenOSC/include/openosc_map.h:21,
                 from /home/openosc-dir/OpenOSC/include/openosc.h:179,
                 from <command-line>:
In function 'strcpy',
    inlined from 'main' at fuzzer2.c:10:2:
/home/openosc-dir/OpenOSC/include/openosc_redirect_map.h:379:35: error: call to 'openosc_strcpy_chk_warn' declared with attribute error: strcpy caller with bigger length than size of destination buffer
  379 |                   : (STRCPY_CASE2 openosc_strcpy_chk_warn(_sz, dst, src)))
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
root@2751ad9120bb:/home/openosc-dir/fuzz# 

Perhaps this is because "char src[] = " defines a memory region on the stack, while "char *src =" defines a pointer which points to memory in read-only section not on the stack?

For comparison, I tried clang-14, which also fails to catch this overflow at compile time.

root@2751ad9120bb:/home/openosc-dir/fuzz# ls -tl fuzzer2.o fuzzer.o
-rw-------. 1 root root 5184 Dec 7 04:02 fuzzer2.o
-rw-------. 1 root root 4992 Dec 7 04:02 fuzzer.o
root@2751ad9120bb:/home/openosc-dir/fuzz# clang --version
Ubuntu clang version 14.0.0-1ubuntu1.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
root@2751ad9120bb:/home/openosc-dir/fuzz#

I will see if I can improve OpenOSC code for this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants