Bug#730487: Proof of concept for corruption
With the attached proof of concept I can cause a crash, but is also
proves that the returned path can change after a second allocation.
It can only happen when someone reuses the allocated buffer and assumes
that strlen(x) gives you the right length for the buffer, however it is
quite odd to not make such an assumption.
In any case, it also proves that the returned string can become invalid
after another allocation; the result of the proof of concept for me is:
24: /tmp/abcdef/.local/share
25: /tmp/abcdef/.local/share!
24: /tmp/abcdef/.local/games
and a crash for invalid free
This shows that after the second allocation the value at the first
pointer has changed. Its length has been increased by 1. This is also
what is most troublesome to me; it will create transient issues with
applications that use these functions where they can't find the data or
config folder in subsequent runs because each time it can return a
different path due to the corruption. However, it would be badly
debuggable as it only happens with certain lengths of user names.
Be aware that there is some code that changes the environment in the
proof of concept that is used to trigger it. On my x86_64 system the
current one triggers it; on other systems it might be different. It all
depends on the alignment of malloc allocations.
#include stdlib.h
#include stdio.h
#include string.h
#include basedir.h
int main(int argc, char *argv[])
{
/* Home needs to be a specific length. In my case the total returned string
* length, including /tmp/abcdef/.local/share needs to be 24. It might be
* different on different architectures. */
setenv(HOME, /tmp/abcdef, 1);
/* Allocate the first string, and print the first. */
const char *xdg_data_home = xdgDataHome(NULL);
printf(%d: %s\n, strlen(xdg_data_home), xdg_data_home);
/* Allocate the second string. */
const char *xdg_config_home = xdgConfigHome(NULL);
/* Now print the first string again; for me it's character byte longer now.
* This also means that we do not place it in the data home directory, but
* yet another directory. Possibly with a random name, making it quite
* unreliable where the files will be. This will eventually cause all kinds
* of transient file cannot be found errors. But only for those that have
* exactly the wrong amount of characters in the $HOME path. */
printf(%d: %s\n, strlen(xdg_data_home), xdg_data_home);
/*
* Now we really start to mess with things...
* We know, from the code, not the documentation, that we have control over
* the data, i.e. the data that was allocated is now ours. For strings it
* would generally be safe to assume that all characters of the string and
* the '\0' belong to the allocated string.
* Imagine we want the share replaced with games to match the games folder
* in /usr/.
*/
char *path = strrchr(xdg_data_home, '/');
if (path != NULL strlen(path + 1) = 5) {
strcpy(path + 1, games);
printf(%d: %s\n, strlen(xdg_data_home), xdg_data_home);
}
/* Now clean everything up. */
free((char*)xdg_data_home); // BOOM
free((char*)xdg_config_home);
return 0;
}
Bug#730487: libxdg-basedir: Writing beyond allocated buffer
Package: libxdg-basedir1 Version: 1.1.1-2 Severity: critical File: libxdg-basedir Tags: patch upstream Dear Maintainer, Any application using xdgDataHome, xdgConfigHome and possibly others will trigger invalid reads and writes in valgrind. For example the following code: const char *xdg_data_home = xdgDataHome(NULL); printf(%s\n, xdg_data_home); It triggers: ==14808== Invalid write of size 1 ==14808==at 0x4C2D97A: memcpy@GLIBC_2.2.5 (mc_replace_strmem.c:881) ==14808==by 0x4E352A0: xdgGetRelativeHome (basedir.c:577) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== Address 0x53e305b is 0 bytes after a block of size 27 alloc'd ==14808==at 0x4C294A0: malloc (vg_replace_malloc.c:291) ==14808==by 0x4E35278: xdgGetRelativeHome (basedir.c:575) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== ==14808== Invalid read of size 1 ==14808==at 0x4C2C954: __GI_strlen (mc_replace_strmem.c:405) ==14808==by 0x50A42DB: puts (ioputs.c:36) ==14808==by 0x4006B5: main (in /tmp/a.out) ==14808== Address 0x53e305b is 0 bytes after a block of size 27 alloc'd ==14808==at 0x4C294A0: malloc (vg_replace_malloc.c:291) ==14808==by 0x4E35278: xdgGetRelativeHome (basedir.c:575) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== The solution is simple: add a +1 to the malloc in line 575 of basedir.c. This as it uses the +1 on line 577 when limiting the amount of data that is to be copied. Attached are: * a full test case * full result of valgrind before patching the library * the patch for the library * full result of valgrind after patching the library I'm in doubt whether this is really this critical or not; I could imagine ways that, under certain circumstances, it is possible to crash users of this library. Especially when this allocation happens at the end of a page, and the next page isn't valid. Then writing the last byte/character could trigger a segmentation fault. Furthermore, the last byte could corrupt the memory management pointers at the begin of allocations causing malloc and/or free to crash, as well as malloc/free overwriting the last byte and the string then becoming longer due to the missing '\0' termination causing applications to crash when e.g. strdup allocates the memory based on strlen before allocation, then allocates and corrupts the string, and finally memcpy-ing but not '\0' terminating the duplicated string. These are all hypothetical though. Regards, Remko Bijker -- System Information: Debian Release: jessie/sid APT prefers unstable APT policy: (500, 'unstable'), (1, 'experimental') Architecture: amd64 (x86_64) Kernel: Linux 3.11-2-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages libxdg-basedir1 depends on: ii libc6 2.17-96 libxdg-basedir1 recommends no packages. libxdg-basedir1 suggests no packages. -- no debconf information ==14838== Memcheck, a memory error detector ==14838== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==14838== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==14838== Command: ./a.out ==14838== /home/rubidium/.local/share ==14838== ==14838== HEAP SUMMARY: ==14838== in use at exit: 0 bytes in 0 blocks ==14838== total heap usage: 1 allocs, 1 frees, 28 bytes allocated ==14838== ==14838== All heap blocks were freed -- no leaks are possible ==14838== ==14838== For counts of detected and suppressed errors, rerun with: -v ==14838== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2) ==14808== Memcheck, a memory error detector ==14808== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==14808== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==14808== Command: ./a.out ==14808== ==14808== Invalid write of size 1 ==14808==at 0x4C2D97A: memcpy@GLIBC_2.2.5 (mc_replace_strmem.c:881) ==14808==by 0x4E352A0: xdgGetRelativeHome (basedir.c:577) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== Address 0x53e305b is 0 bytes after a block of size 27 alloc'd ==14808==at 0x4C294A0: malloc (vg_replace_malloc.c:291) ==14808==by 0x4E35278: xdgGetRelativeHome (basedir.c:575) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== ==14808== Invalid read of size 1 ==14808==at 0x4C2C954: __GI_strlen (mc_replace_strmem.c:405) ==14808==by 0x50A42DB: puts (ioputs.c:36) ==14808==by 0x4006B5: main (in /tmp/a.out) ==14808== Address 0x53e305b is 0 bytes after a block of size 27 alloc'd ==14808==at 0x4C294A0: malloc (vg_replace_malloc.c:291) ==14808==by 0x4E35278: xdgGetRelativeHome (basedir.c:575) ==14808==by 0x4006A5: main (in /tmp/a.out) ==14808== /home/rubidium/.local/share ==14808== ==14808== HEAP SUMMARY: ==14808== in use at exit: 0 bytes in 0 blocks ==14808== total heap usage: 1 allocs, 1 frees, 27 bytes allocated ==14808== ==14808== All heap blocks were freed --
Bug#493714: openttd: Network exploitable buffer overrun
Moritz Muehlenhoff wrote: I agree, can you upload 0.6.2 to unstable? Cheers, Moritz I have absolutely no influence over getting OpenTTD 0.6.2 physically uploaded to Debian-unstable. Regards, Remko Bijker -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#493714: openttd: Network exploitable buffer overrun
Package: openttd Version: 0.6.1-1 Severity: grave Tags: security Justification: user security hole OpenTTD servers of version 0.6.1 and below are susceptible to a remotely exploitable buffer overflow when the server is filled with companies and clients with names that are (near) the maximum allowed length for names. In the worst case OpenTTD will write the following (mostly remotely changable bytes) into 1460 bytes of malloc-ed memory: up to 11 times (amount of players) 118 bytes up to 8 times (amount of companies) 124 bytes and 7 header bytes Resulting in up to 2297 bytes being written in 1460 bytes of malloc-ed memory. This makes it possible to remotely crash the game or change the gamestate into an unrecoverable state. There are three ways of fixing this: - upgrading to 0.6.2. - backporting the bugfixes to 0.6.1 and make a network-incompatible version of OpenTTD which makes it impossible to participate in multiplayer games with both Debian and non-Debian users. - increase the allocation size, which will make it even network incompatible with itself. Therefore the best way to fix this is by upgrading to 0.6.2, also in lenny. -- System Information: Debian Release: lenny/sid APT prefers unstable APT policy: (500, 'unstable') Architecture: i386 (i686) Kernel: Linux 2.6.26 (PREEMPT) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_GB.utf8) Shell: /bin/sh linked to /bin/bash -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
