[
https://issues.apache.org/jira/browse/COMPRESS-688?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Zaki updated COMPRESS-688:
--------------------------
Description:
h2. Reporting a bug found by iCR
In file:
[SevenZFile.java|https://github.com/apache/commons-compress/blob/master/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java#L1252],
there is a potential case of null pointer dereference. In method
*readFilesInfo* inside class {*}SevenZFile{*}, there are calls to *readBits*
inside *NID.kEmptyFile* and *NID.kAnti* cases of the switch statement. One of
the parameters passed to the method is {*}isEmptyStream.cardinality(){*}.
*isEmptyStream* is initialized to null and this can lead to
*NullPointerException* if *NID.kEmptyFile* or *NID.kAnti* case is encountered
before *NID.kEmptyStream* which assigns value to {*}isEmptyStream{*}. So iCR
suggests proper null checking before invoking `{*}cardinality(){*}` on
{*}isEmptyStream{*}.
{code:java}
private void readFilesInfo(final ByteBuffer header, final Archive archive)
throws IOException {
final int numFilesInt = (int) readUint64(header);
final Map<Integer, SevenZArchiveEntry> fileMap = new LinkedHashMap<>();
BitSet isEmptyStream = null;
BitSet isEmptyFile = null;
BitSet isAnti = null;
while (true) {
final int propertyType = getUnsignedByte(header);
if (propertyType == 0) {
break;
}
final long size = readUint64(header);
switch (propertyType) {
case NID.kEmptyStream: {
isEmptyStream = readBits(header, numFilesInt);
break;
}
case NID.kEmptyFile: {
isEmptyFile = readBits(header, isEmptyStream.cardinality());
break;
}
case NID.kAnti: {
isAnti = readBits(header, isEmptyStream.cardinality());
break;
}
...
}{code}
It is not immediately clear whether *NID.kEmptyStream* would always be entered
before either *NID.kEmptyFile* or {*}NID.kAnti{*}. If that is indeed the case
then this issue can be ignored at your discretion.
h3. Sponsorship and Support
This work is done by the security researchers from OpenRefactory and is
supported by the [Open Source Security Foundation
(OpenSSF)|https://openssf.org/]: [Project
Alpha-Omega|https://alpha-omega.dev/]. Alpha-Omega is a project partnering with
open source software project maintainers to systematically find new,
as-yet-undiscovered vulnerabilities in open source code - and get them fixed -
to improve global software supply chain security.
The bug is found by running the iCR tool by [OpenRefactory,
Inc.|https://openrefactory.com/] and then manually triaging the results.
was:h2. Reporting a bug found by iCR
> Potential Null Pointer Dereference in SevenZFile.java
> -----------------------------------------------------
>
> Key: COMPRESS-688
> URL: https://issues.apache.org/jira/browse/COMPRESS-688
> Project: Commons Compress
> Issue Type: Bug
> Affects Versions: 1.26.0, 1.26.1, 1.26.2, 1.27.1
> Reporter: Zaki
> Priority: Minor
> Attachments: bug-report.pdf
>
>
> h2. Reporting a bug found by iCR
> In file:
> [SevenZFile.java|https://github.com/apache/commons-compress/blob/master/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java#L1252],
> there is a potential case of null pointer dereference. In method
> *readFilesInfo* inside class {*}SevenZFile{*}, there are calls to *readBits*
> inside *NID.kEmptyFile* and *NID.kAnti* cases of the switch statement. One of
> the parameters passed to the method is {*}isEmptyStream.cardinality(){*}.
> *isEmptyStream* is initialized to null and this can lead to
> *NullPointerException* if *NID.kEmptyFile* or *NID.kAnti* case is encountered
> before *NID.kEmptyStream* which assigns value to {*}isEmptyStream{*}. So iCR
> suggests proper null checking before invoking `{*}cardinality(){*}` on
> {*}isEmptyStream{*}.
>
> {code:java}
> private void readFilesInfo(final ByteBuffer header, final Archive
> archive) throws IOException {
> final int numFilesInt = (int) readUint64(header);
> final Map<Integer, SevenZArchiveEntry> fileMap = new
> LinkedHashMap<>();
> BitSet isEmptyStream = null;
> BitSet isEmptyFile = null;
> BitSet isAnti = null;
> while (true) {
> final int propertyType = getUnsignedByte(header);
> if (propertyType == 0) {
> break;
> }
> final long size = readUint64(header);
> switch (propertyType) {
> case NID.kEmptyStream: {
> isEmptyStream = readBits(header, numFilesInt);
> break;
> }
> case NID.kEmptyFile: {
> isEmptyFile = readBits(header, isEmptyStream.cardinality());
> break;
> }
> case NID.kAnti: {
> isAnti = readBits(header, isEmptyStream.cardinality());
> break;
> }
>
> ...
>
> }{code}
>
> It is not immediately clear whether *NID.kEmptyStream* would always be
> entered before either *NID.kEmptyFile* or {*}NID.kAnti{*}. If that is indeed
> the case then this issue can be ignored at your discretion.
> h3. Sponsorship and Support
> This work is done by the security researchers from OpenRefactory and is
> supported by the [Open Source Security Foundation
> (OpenSSF)|https://openssf.org/]: [Project
> Alpha-Omega|https://alpha-omega.dev/]. Alpha-Omega is a project partnering
> with open source software project maintainers to systematically find new,
> as-yet-undiscovered vulnerabilities in open source code - and get them fixed
> - to improve global software supply chain security.
> The bug is found by running the iCR tool by [OpenRefactory,
> Inc.|https://openrefactory.com/] and then manually triaging the results.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
