lib/58136: Use after free in libintl pgettext

[stix%stix.id.au@localhost]

>Number:         58136
>Category:       lib
>Synopsis:       Use after free in libintl pgettext
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 10 07:25:00 +0000 2024
>Originator:     Paul Ripke
>Release:        NetBSD 10.0_RC6
>Organization:
Paul Ripke
"Great minds discuss ideas, average minds discuss events, small minds
 discuss people."
-- Disputed: Often attributed to Eleanor Roosevelt. 1948.
>Environment:
System: NetBSD slave 10.0_RC6 NetBSD 10.0_RC6 (SLAVE) #8: Sun Mar 17 09:38:38 AEDT 2024 stix@slave:/home/netbsd/netbsd-10/obj.amd64/home/netbsd/netbsd-10/src/sys/arch/amd64/compile/SLAVE amd64
Architecture: x86_64
Machine: amd64
>Description:
While attempting to port the latest release of games/widelands, running under
ASAN generated the following:

==1020==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000271f30 at pc 0x7f7ff6e4c10f bp 0x7f7fffff5d80 sp 0x7f7fffff5530
READ of size 9 at 0x603000271f30 thread T0
    #0 0x7f7ff6e4c10e  (/usr/lib/libasan.so.5+0x4c10e)
    #1 0x7f7ff2604e80  (/usr/lib/libintl.so.1+0x4e80)
    #2 0x1db76d1 in pgettext_wrapper (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1bb76d1)
    #3 0xd7f351 in UI::BaseDropdown::BaseDropdown(UI::Panel*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, int, int, unsigned int, unsigned int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, UI::DropdownType, UI::PanelStyle, UI::ButtonStyle) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xb7f351)
    #4 0xe7f4e7 in FsMenu::MainMenu::MainMenu(bool) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xc7f4e7)
    #5 0x89c749 in WLApplication::run() (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x69c749)
    #6 0x1fba7b9 in main (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1dba7b9)
    #7 0x86020c in ___start (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x66020c)

0x603000271f30 is located 0 bytes inside of 21-byte region [0x603000271f30,0x603000271f45)
freed by thread T0 here:
    #0 0x7f7ff6e433e7 in __interceptor_free (/usr/lib/libasan.so.5+0x433e7)
    #1 0x7f7ff2604e73  (/usr/lib/libintl.so.1+0x4e73)
    #2 0x1db76d1 in pgettext_wrapper (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1bb76d1)
    #3 0xd7f351 in UI::BaseDropdown::BaseDropdown(UI::Panel*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, int, int, unsigned int, unsigned int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, UI::DropdownType, UI::PanelStyle, UI::ButtonStyle) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xb7f351)
    #4 0xe7f4e7 in FsMenu::MainMenu::MainMenu(bool) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xc7f4e7)
    #5 0x89c749 in WLApplication::run() (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x69c749)
    #6 0x1fba7b9 in main (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1dba7b9)
    #7 0x86020c in ___start (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x66020c)
    #8 0x7f7ff7c0bbb7  (/usr/libexec/ld.elf_so+0xbbb7)

previously allocated by thread T0 here:
    #0 0x7f7ff6e43a5a in __interceptor_realloc (/usr/lib/libasan.so.5+0x43a5a)
    #1 0x7f7ff073afc5 in vasprintf_l (/lib/libc.so.12+0x13afc5)
    #2 0x7f7ff6eb1e17 in __interceptor_vasprintf (/usr/lib/libasan.so.5+0xb1e17)
    #3 0x7f7ff6eb25c2 in __interceptor_asprintf (/usr/lib/libasan.so.5+0xb25c2)
    #4 0x7f7ff2604e3e  (/usr/lib/libintl.so.1+0x4e3e)
    #5 0x1db76d1 in pgettext_wrapper (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1bb76d1)
    #6 0xd7f351 in UI::BaseDropdown::BaseDropdown(UI::Panel*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, int, int, unsigned int, unsigned int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, UI::DropdownType, UI::PanelStyle, UI::ButtonStyle) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xb7f351)
    #7 0xe7f4e7 in FsMenu::MainMenu::MainMenu(bool) (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0xc7f4e7)
    #8 0x89c749 in WLApplication::run() (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x69c749)
    #9 0x1fba7b9 in main (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x1dba7b9)
    #10 0x86020c in ___start (/home/tmp/pkgwrk/games/widelands/work/widelands/./build/src/widelands+0x66020c)
    #11 0x7f7ff7c0bbb7  (/usr/libexec/ld.elf_so+0xbbb7)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/libasan.so.5+0x4c10e) 
Shadow bytes around the buggy address:
  0x0c0680046390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06800463a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06800463b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06800463c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
  0x0c06800463d0: fd fd fa fa fd fd fd fa fa fa fd fd fd fd fa fa
=>0x0c06800463e0: fd fd fd fa fa fa[fd]fd fd fa fa fa fd fd fd fa
  0x0c06800463f0: fa fa fd fd fd fd fa fa fd fd fd fa fa fa fd fd
  0x0c0680046400: fd fd fa fa 00 00 00 fa fa fa 00 00 04 fa fa fa
  0x0c0680046410: 00 00 00 fa fa fa fd fd fd fa fa fa fd fd fd fd
  0x0c0680046420: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x0c0680046430: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1020==ABORTING

Looking at the code, concatenate_ctxt_id allocates a new string in msgctxt_id.
dcngettext may then return a pointer back to msgctxt_id, which we free, and
then optionally return back to the caller.

>How-To-Repeat:

I'm no expert here, but I think any call to pgettext that doesn't find a
translation may return a pointer to freed memory.

>Fix:

The following patch seems to work, and allowed running games/widelands under
ASAN:

diff --git a/lib/libintl/gettext.c b/lib/libintl/gettext.c
index 424abbd2f567..1bd62a31d4dd 100644
--- a/lib/libintl/gettext.c
+++ b/lib/libintl/gettext.c
@@ -174,11 +174,13 @@ pgettext_impl(const char *domainname, const char *msgctxt, const char *msgid1,
 
 	translation = dcngettext(domainname, msgctxt_id,
 		msgid2, n, category);
-	free(msgctxt_id);
 
 	p = strchr(translation, '\004');
-	if (p)
-		return p + 1;
+	if (p) {
+		free(msgctxt_id);
+		return msgid1;
+	}
+	free(msgctxt_id);
 	return translation;
 }