Mark Wielaard [Thu, 26 May 2022 20:38:01 +0000 (22:38 +0200)]
Initialize the fave and cost arrays fully
We try to be smart in sendMTFValues by initializing just nGroups
number of elements instead of all BZ_N_GROUPS elements. But this means
the compiler doesn't know all elements are correctly initialized and
might warn. The arrays are really small, BZ_N_GROUPS, 6 elements. And
nGroups == BZ_N_GROUPS is the common case. So just initialize them all
always. Using a constant loop might also help the compiler to optimize
the initialization.
Mark Wielaard [Sun, 17 May 2020 18:02:31 +0000 (20:02 +0200)]
Don't call unsafe functions from SIGSEGV/SIGBUS signal handler.
GCC10 -fanalyzer notices that we try to call functions that are not
signal safe from our fatal signal handler:
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:819:7: warning: call to ‘fprintf’ from within signal handler
[CWE-479] [-Wanalyzer-unsafe-call-within-signal-handler]
It also notices we then call showFileNames and cleanupAndFail which
also call possibly not signal safe functions.
Just write out the error message directly to STDERR and exit without
trying to clean up any files.
Mark Wielaard [Tue, 2 Jul 2019 23:28:11 +0000 (01:28 +0200)]
Accept as many selectors as the file format allows.
But ignore any larger than the theoretical maximum, BZ_MAX_SELECTORS.
The theoretical maximum number of selectors depends on the maximum
blocksize (900000 bytes) and the number of symbols (50) that can be
encoded with a different Huffman tree. BZ_MAX_SELECTORS is 18002.
But the bzip2 file format allows the number of selectors to be encoded
with 15 bits (because 18002 isn't a factor of 2 and doesn't fit in
14 bits). So the file format maximum is 32767 selectors.
Some bzip2 encoders might actually have written out more selectors
than the theoretical maximum because they rounded up the number of
selectors to some convenient factor of 8.
The extra 14766 selectors can never be validly used by the decompression
algorithm. So we can read them, but then discard them.
This is effectively what was done (by accident) before we added a
check for nSelectors to be at most BZ_MAX_SELECTORS to mitigate
CVE-2019-12900.
The extra selectors were written out after the array inside the
EState struct. But the struct has extra space allocated after the
selector arrays of 18060 bytes (which is larger than 14766).
All of which will be initialized later (so the overwrite of that
space with extra selector values would have been harmless).
Changes the include path separator for Windows builds to use "/" instead
of "\". Windows has no problems with using a forward slash as a path
separator, but using a backslash causes problems when attempting to
cross compile for other platforms (for example, when trying to cross
compile for MinGW from Linux).
Mark Wielaard [Tue, 25 Jun 2019 17:22:37 +0000 (19:22 +0200)]
Add prepare-release.sh script.
Script to run to prepare a new release.
It will update the release number and tell you to update the
CHANGES file and to double check everything looks before doing
the release commit and tagging.
Afterwards you probably want to run release-update.sh to upload
the release and update the website at https://sourceware.org/bzip2/
There are embedded version strings and dates in a couple of places.
To keep the script simple remove some that aren't absolutely necessary.
README now just points to CHANGES.
README.COMPILATION.PROBLEMS only mentions the version once at the top.
bzip2.c only mentions the version once when doing --version.
manual.xml now doesn't have any embedded versions, just uses &bz-version;
everywhere.
nSelectors is used in a loop from 0 to nSelectors to access selectorMtf
which is
UChar selectorMtf[BZ_MAX_SELECTORS];
so if nSelectors is bigger than BZ_MAX_SELECTORS it'll do an invalid memory
access
Fixes out of bounds access discovered while fuzzying karchive
This was reported as CVE-2019-12900
BZ2_decompress in decompress.c in bzip2 through 1.0.6 has an
out-of-bounds write when there are many selectors.
Paul Kehrer [Sat, 8 Jun 2019 14:06:40 +0000 (10:06 -0400)]
Fix undefined behavior in the macros SET_BH, CLEAR_BH, & ISSET_BH
These macros contain this pattern:
1 << ((Int32_value) & 31
This causes the undefined behavior sanitizers in clang and gcc to
complain because the shift, while ultimately stored to an unsigned
variable, is done as a signed value. Adding a cast to unsigned for
the int32 value resolves this issue.
Mark Wielaard [Mon, 24 Jun 2019 07:31:16 +0000 (09:31 +0200)]
bzip2: Fix return value when combining --test,-t and -q.
When passing -q to get quiet output --test would not display an error
message, but would also suppress the exit 2 code to indicate the file
was corrupt. Only suppress the error message with -q, not the exit value.
This patch comes from Debian.
"bunzip2 -qt returns 0 for corrupt archives"
https://bugs.debian.org/279025
Mark Wielaard [Sun, 23 Jun 2019 21:52:03 +0000 (23:52 +0200)]
bzip2.c (testStream): Remove set, but not used nread variable.
Modern GCC warns:
bzip2.c: In function ‘testStream’:
bzip2.c:557:37: warning: variable ‘nread’ set but not used
[-Wunused-but-set-variable]
Int32 bzerr, bzerr_dummy, ret, nread, streamNo, i;
^~~~~
GCC is correct. In testStream we don't care about the number of bytes
read by BZ2_bzRead. So just remove the variable and the assignment.
Mark Wielaard [Sun, 23 Jun 2019 20:18:58 +0000 (22:18 +0200)]
Add release-update.sh script.
Script to run after a release has been tagged, signed and pushed
to git. Will do a fresh checkout, verify the git tag, do fresh
build/dist, sign the dist with gpg, create a backup copy in HOME,
upload the tar.gz and sig to sourceware, checkout bzip2-htdocs,
copy over the new changes, manual, etc. and git push that to update
https://sourceware.org/bzip2/