Peter Harris authored 4 months ago and Alan Coopersmith committed 4 months ago

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <!25>

v2: Exclude include of <strings.h> as well

Fixes: 73b69d79 ("Remove xpmstrcasecmp fallback for strcasecmp")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <!25>

It has never been used in the autoconf builds

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

It was only ever used when FOR_MSW was defined, never in the autoconf builds

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Removes all the "FOR_MSW" code added back in 1994, which hasn't been
buildable in the automake builds.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

There was no way to build this in the automake files

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Assumes all platforms have provided C99 required headers in the 25 years
since the C99 standard was finalized.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Yair Mizrahi authored 1 year ago and Alan Coopersmith committed 1 year ago

This doesn't fix the CVE - that has to happen in libX11, this
just tries to avoid triggering it from libXpm, and saves time
in not pretending we can successfully create an X Image for
which the width * depth would overflow the signed int used to
store the bytes_per_line value.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Provided by Yair Mizrahi of the JFrog Vulnerability Research team

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

This doesn't fix the CVE - that has to happen in libX11, this
just tries to avoid triggering it from libXpm, and saves time
in not pretending we can successfully create an X11 pixmap with
dimensions larger than the unsigned 16-bit integers used in the
X11 protocol for the dimensions.

Reported by Yair Mizrahi of the JFrog Vulnerability Research team

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Provided by Yair Mizrahi of the JFrog Vulnerability Research team

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Found with clang's libfuzzer

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Generated by clang's -fsanitize/libfuzzer

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

When the test case for CVE-2022-46285 was run with the Address Sanitizer
enabled, it found an out-of-bounds read in ParseComment() when reading
from a memory buffer instead of a file, as it continued to look for the
closing comment marker past the end of the buffer.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Hides private API from external linkage

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

g_pattern_spec_match_string was introduced in glib 2.70 to replace
g_pattern_match_string which is deprecated in glib 2.70 and later.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Relies on platforms with O_CLOEXEC support following POSIX requirement
to not copy the close-on-exec flag to the new fd in dup2(), but to leave
it unset instead, since that's how fd's are passed to child processes
to handled compressed files.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Matt Turner <mattst88@gmail.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

parse.c:74:1: warning: unused function 'xstrlcpy' [-Wunused-function]
xstrlcpy(char *dst, const char *src, size_t dstsize)
^

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Makes it match the definition in create.c and eliminates
clang warnings:

create.c:2409:13: warning: empty expression statement has no effect;
 remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
                        FREE_CIDX;
                                 ^
create.c:2440:17: warning: empty expression statement has no effect;
 remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
                            FREE_CIDX;
                                     ^
create.c:2444:13: warning: empty expression statement has no effect;
 remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
                        FREE_CIDX;
                                 ^
create.c:2449:15: warning: empty expression statement has no effect;
 remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
            FREE_CIDX;
                     ^

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

CrDatFrI.c: In function ‘XpmCreateDataFromXpmImage’:
CrDatFrI.c:245:13: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
  245 |             if (header[l])
      |             ^~
In file included from CrDatFrI.c:40:
XpmI.h:80:22: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
   80 | #define XpmFree(ptr) free(ptr)
      |                      ^~~~
CrDatFrI.c:247:17: note: in expansion of macro ‘XpmFree’
  247 |                 XpmFree(header);
      |                 ^~~~~~~
CrDatFrI.c: In function ‘CreateColors’:

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

AC_PROG_LIBTOOL was replaced by LT_INIT in libtool 2 in 2008,
so it's time to rely on it.

configure.ac:14: warning: The macro `AC_PROG_LIBTOOL' is obsolete.
configure.ac:14: You should run autoupdate.
m4/libtool.m4:100: AC_PROG_LIBTOOL is expanded from...
configure.ac:14: the top level

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

If compress is not found, we disable writing to .Z files,
but leave the rest of the compression code active.

If uncompress is not found, we use gzip to read .Z files.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Peter Hutterer authored 2 years ago and Alan Coopersmith committed 2 years ago

In this particular error path we have already allocated cidx[0..256]
with 256 instances of fresh and juicy memory. Freeing that is annoying,
but luckily there's a helpful FREE_CIDX macro that does exactly that.

Fixes f80fa6ae:
  Fix CVE-2022-44617: Runaway loop with width of 0 and enormous height

Found by covscan

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

When one of the compression helper programs is not found, the message
suggesting how to compile without it should say --disable-open-zfile,
not --disable-stat-zfile.

Fixes: 515294bb ("Fix CVE-2022-4883: compression commands depend on $PATH")
Closes: #4

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Reported-by: T.J. Townsend
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Peter Hutterer authored 2 years ago and Alan Coopersmith committed 2 years ago

GNU gunzip [1] is a shell script that exec's `gzip -d`. Even if we call
/usr/bin/gunzip with the correct built-in path, the actual gzip call
will use whichever gzip it finds first, making our patch pointless.

Fix this by explicitly calling gzip -d instead.

https://git.savannah.gnu.org/cgit/gzip.git/tree/gunzip.in



[Part of the fix for CVE-2022-4883]
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Matthieu Herrb authored 2 years ago and Alan Coopersmith committed 2 years ago

xpmParseDataAndCreate() calls XDestroyImage() in the error path.
Reproducible with sxpm "zero-width.xpm", that file is in the test/
directory.

The same approach is needed in the bytes_per_line == 0 condition though
here it just plugs a memory leak.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

By default, on all platforms except MinGW, libXpm will detect if a
filename ends in .Z or .gz, and will when reading such a file fork off
an uncompress or gunzip command to read from via a pipe, and when
writing such a file will fork off a compress or gzip command to write
to via a pipe.

In libXpm 3.5.14 or older these are run via execlp(), relying on $PATH
to find the commands.  If libXpm is called from a program running with
raised privileges, such as via setuid, then a malicious user could set
$PATH to include programs of their choosing to be run with those
privileges.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

When reading XPM images from a file with libXpm 3.5.14 or older, if a
image has a width of 0 and a very large height, the ParsePixels() function
will loop over the entire height calling getc() and ungetc() repeatedly,
or in some circumstances, may loop seemingly forever, which may cause a
denial of service to the calling program when given a small crafted XPM
file to parse.

Closes: #2

Reported-by: Martin Ettl <ettl.martin78@googlemail.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

When reading XPM images from a file with libXpm 3.5.14 or older, if a
comment in the file is not closed (i.e. a C-style comment starts with
"/*" and is missing the closing "*/"), the ParseComment() function will
loop forever calling getc() to try to read the rest of the comment,
failing to notice that it has returned EOF, which may cause a denial of
service to the calling program.

Reported-by: Marco Ivaldi <raptor@0xdeadbeef.info>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>