Skip to content

Commit

Permalink
protexec: fix out-of-bounds stack write
Browse files Browse the repository at this point in the history
In create_tempfile() we look for a suitable place to put the temporary
file into and, among others, look at $TMPDIR. If the value of this
environment variable exceeds the bounds of the local tmp_name[] buffer,
we ignore it. However, we still change the value of 'tmp_name_len' which
leads to follow-up errors.

On debug builds this can lead to hitting the assertion as can be seen
below:

$ TMPDIR=$(perl -e 'print "A"x1024') ./bin/array_access
Assertion failed at sljit_src/sljitProtExecAllocator.c:147
Aborted

For non-debug builds, however, this can lead to a memory corruption, by
abusing the fact that we change a trailing '/' to '\0' later on. With a
sufficiently high enough value for 'tmp_name_len' this can corrupt stack
frames up in the call chain.

Fix this by setting 'tmp_name_len' only if value it is based on is found
to be valid -- just like it was prior to commit 98323bd.

Fixes: 98323bd ("protexec: refactor create_tempfile() (#37)")
Signed-off-by: Mathias Krause <[email protected]>
  • Loading branch information
minipli-oss committed Nov 8, 2022
1 parent 8996588 commit 28b952b
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions sljit_src/sljitProtExecAllocator.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ static SLJIT_INLINE int create_tempfile(void)
int fd;
char tmp_name[256];
size_t tmp_name_len = 0;
size_t tmp_len;
char *dir;
struct stat st;
#if defined(SLJIT_SINGLE_THREADED) && SLJIT_SINGLE_THREADED
Expand All @@ -125,18 +126,22 @@ static SLJIT_INLINE int create_tempfile(void)
dir = secure_getenv("TMPDIR");

if (dir) {
tmp_name_len = strlen(dir);
if (tmp_name_len > 0 && tmp_name_len < sizeof(tmp_name)) {
if ((stat(dir, &st) == 0) && S_ISDIR(st.st_mode))
tmp_len = strlen(dir);
if (tmp_len > 0 && tmp_len < sizeof(tmp_name)) {
if ((stat(dir, &st) == 0) && S_ISDIR(st.st_mode)) {
strcpy(tmp_name, dir);
tmp_name_len = tmp_len;
}
}
}

#ifdef P_tmpdir
if (!tmp_name_len) {
tmp_name_len = strlen(P_tmpdir);
if (tmp_name_len > 0 && tmp_name_len < sizeof(tmp_name))
tmp_len = strlen(P_tmpdir);
if (tmp_len > 0 && tmp_len < sizeof(tmp_name)) {
strcpy(tmp_name, P_tmpdir);
tmp_name_len = tmp_len;
}
}
#endif
if (!tmp_name_len) {
Expand Down

0 comments on commit 28b952b

Please sign in to comment.