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

substr: fix buffer overflow with utf-8 strings #205

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

millert
Copy link
Contributor

@millert millert commented Oct 28, 2023

We need to use u8_strlen(), not strlen(), to compute the length. Otherwise, there may be an out of bounds write when writing the NUL terminator to set the length of the substring.

We need to use u8_strlen(), not strlen(), to compute the length.
Otherwise, there may be an out of bounds write when writing the NUL
terminator to set the length of the substring.
@millert
Copy link
Contributor Author

millert commented Oct 28, 2023

The following awk script shows the problem when with valgrind, ASAN or the OpenBSD malloc.

BEGIN { name=substr("%A Lukas Bäuerle",3) }

Valgrind ouput before the fix:

==945916== Memcheck, a memory error detector
==945916== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==945916== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==945916== Command: ./a.out BEGIN\ {\ name=substr("%A\ Lukas\ B__uerle",3)\ }
==945916==
==945916== Invalid read of size 1
==945916==    at 0x119BDA: substr (run.c:1020)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11B706: execute (run.c:151)
==945916==    by 0x11B706: assign (run.c:1537)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x1182A8: execute (run.c:151)
==945916==    by 0x1182A8: program (run.c:189)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11D950: execute (run.c:151)
==945916==    by 0x11D950: run (run.c:141)
==945916==    by 0x10CCD6: main (main.c:230)
==945916==  Address 0x4b5ea62 is 0 bytes after a block of size 18 alloc'd
==945916==    at 0x4843828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==945916==    by 0x49EF78D: strdup (strdup.c:42)
==945916==    by 0x11302E: tostring (tran.c:526)
==945916==    by 0x11302E: setsymtab (tran.c:236)
==945916==    by 0x11EBBB: string (lex.c:480)
==945916==    by 0x10D339: yyparse (awkgram.tab.c:2251)
==945916==    by 0x10CC8D: main (main.c:219)
==945916==
==945916== Invalid write of size 1
==945916==    at 0x119BDF: substr (run.c:1021)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11B706: execute (run.c:151)
==945916==    by 0x11B706: assign (run.c:1537)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x1182A8: execute (run.c:151)
==945916==    by 0x1182A8: program (run.c:189)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11D950: execute (run.c:151)
==945916==    by 0x11D950: run (run.c:141)
==945916==    by 0x10CCD6: main (main.c:230)
==945916==  Address 0x4b5ea62 is 0 bytes after a block of size 18 alloc'd
==945916==    at 0x4843828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==945916==    by 0x49EF78D: strdup (strdup.c:42)
==945916==    by 0x11302E: tostring (tran.c:526)
==945916==    by 0x11302E: setsymtab (tran.c:236)
==945916==    by 0x11EBBB: string (lex.c:480)
==945916==    by 0x10D339: yyparse (awkgram.tab.c:2251)
==945916==    by 0x10CC8D: main (main.c:219)
==945916==
==945916== Invalid write of size 1
==945916==    at 0x119BF1: substr (run.c:1023)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11B706: execute (run.c:151)
==945916==    by 0x11B706: assign (run.c:1537)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x1182A8: execute (run.c:151)
==945916==    by 0x1182A8: program (run.c:189)
==945916==    by 0x118180: execute.part.0 (run.c:166)
==945916==    by 0x11D950: execute (run.c:151)
==945916==    by 0x11D950: run (run.c:141)
==945916==    by 0x10CCD6: main (main.c:230)
==945916==  Address 0x4b5ea62 is 0 bytes after a block of size 18 alloc'd
==945916==    at 0x4843828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==945916==    by 0x49EF78D: strdup (strdup.c:42)
==945916==    by 0x11302E: tostring (tran.c:526)
==945916==    by 0x11302E: setsymtab (tran.c:236)
==945916==    by 0x11EBBB: string (lex.c:480)
==945916==    by 0x10D339: yyparse (awkgram.tab.c:2251)
==945916==    by 0x10CC8D: main (main.c:219)
==945916==
==945916==
==945916== HEAP SUMMARY:
==945916==     in use at exit: 32,211 bytes in 276 blocks
==945916==   total heap usage: 325 allocs, 49 frees, 43,969 bytes allocated
==945916==
==945916== LEAK SUMMARY:
==945916==    definitely lost: 1 bytes in 1 blocks
==945916==    indirectly lost: 0 bytes in 0 blocks
==945916==      possibly lost: 0 bytes in 0 blocks
==945916==    still reachable: 32,210 bytes in 275 blocks
==945916==         suppressed: 0 bytes in 0 blocks
==945916== Rerun with --leak-check=full to see details of leaked memory
==945916==
==945916== For lists of detected and suppressed errors, rerun with: -s
==945916== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

bob-beck pushed a commit to openbsd/src that referenced this pull request Oct 29, 2023
We need to use u8_strlen(), not strlen(), to compute the length.
Otherwise, there may be an out of bounds write when writing the NUL
terminator to set the length of the substring.
onetrueawk/awk#205
@plan9 plan9 merged commit c7361cd into onetrueawk:staging Oct 31, 2023
@millert millert deleted the substr_overflow branch October 31, 2023 01:04
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

Successfully merging this pull request may close these issues.

None yet

2 participants