Bug 66425: Avoid NullPointerExceptions and ClassCastExceptions found via poi-fuzz

We try to avoid throwing NullPointerException and ClassCastExceptions, but it was possible
to trigger them

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62414
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62442
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62450

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912365 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2023-09-17 14:38:24 +00:00
parent 4b520ff7c5
commit 9e2ce70d2b
16 changed files with 91 additions and 17 deletions

View File

@ -21,6 +21,7 @@ import org.apache.xmlbeans.XmlCursor;
import org.apache.xmlbeans.XmlObject;
import org.openxmlformats.schemas.wordprocessingml.x2006.main.*;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
@ -361,7 +362,8 @@ public class XWPFComment implements IBody {
* @return string id
*/
public String getId() {
return ctComment.getId().toString();
final BigInteger id = ctComment.getId();
return id == null ? "-1" : id.toString();
}
/**

View File

@ -17,6 +17,7 @@
package org.apache.poi.hslf.dev;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
@ -26,7 +27,6 @@ import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import org.apache.commons.io.output.StringBuilderWriter;
import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
import org.apache.poi.hslf.record.RecordTypes;
import org.apache.poi.hslf.usermodel.HSLFSlideShow;
@ -122,6 +122,11 @@ public final class PPTXMLDump {
int size = (int)LittleEndian.getUInt(data, pos);
pos += LittleEndianConsts.INT_SIZE;
if (size < 0) {
// stop processing of invalid header data
continue;
}
//get name of the record by type
String recname = RecordTypes.forTypeID(type).name();
write(out, "<"+recname + " info=\""+info+"\" type=\""+type+"\" size=\""+size+"\" offset=\""+(pos-8)+"\"", padding);
@ -214,12 +219,10 @@ public final class PPTXMLDump {
dump.dump(out);
out.close();
} else {
StringBuilderWriter out = new StringBuilderWriter(1024);
dump.dump(out);
System.out.println(out);
dump.dump(new BufferedWriter(
new OutputStreamWriter(System.out, StandardCharsets.UTF_8)));
}
}
}
}

View File

@ -194,6 +194,11 @@ public final class SlideShowDumper {
pos += 8;
out.printf(Locale.ROOT, ind + "That's a %2$s%n", "", recordName);
if (len < 0 /*|| len > Integer.MAX_VALUE*/) {
// stop processing of invalid header data
continue;
}
// Now check if it's a container or not
int container = opt & 0x0f;
@ -219,7 +224,7 @@ public final class SlideShowDumper {
}
}
pos += (int) len;
pos += (int) Math.min(len, Integer.MAX_VALUE);
}
}

View File

@ -26,8 +26,10 @@ import java.util.function.Supplier;
import org.apache.poi.EncryptedDocumentException;
import org.apache.poi.poifs.crypt.CipherAlgorithm;
import org.apache.poi.poifs.crypt.EncryptionHeader;
import org.apache.poi.poifs.crypt.EncryptionInfo;
import org.apache.poi.poifs.crypt.EncryptionMode;
import org.apache.poi.poifs.crypt.EncryptionVerifier;
import org.apache.poi.poifs.crypt.HashAlgorithm;
import org.apache.poi.poifs.crypt.cryptoapi.CryptoAPIEncryptionHeader;
import org.apache.poi.poifs.crypt.cryptoapi.CryptoAPIEncryptionVerifier;
@ -118,8 +120,16 @@ public final class DocumentEncryptionAtom extends PositionDependentRecordAtom {
bos.writeShort(ei.getVersionMinor());
bos.writeInt(ei.getEncryptionFlags());
((CryptoAPIEncryptionHeader)ei.getHeader()).write(bos);
((CryptoAPIEncryptionVerifier)ei.getVerifier()).write(bos);
final EncryptionHeader header = ei.getHeader();
if (!(header instanceof CryptoAPIEncryptionHeader)) {
throw new IllegalStateException("Had unexpected type of header: " + header.getClass());
}
((CryptoAPIEncryptionHeader) header).write(bos);
final EncryptionVerifier verifier = ei.getVerifier();
if (!(verifier instanceof CryptoAPIEncryptionVerifier)) {
throw new IllegalStateException("Had unexpected type of verifier: " + verifier.getClass());
}
((CryptoAPIEncryptionVerifier) verifier).write(bos);
// Header
LittleEndian.putInt(_header, 4, bos.getWriteIndex());

View File

@ -64,6 +64,7 @@ public abstract class BaseTestPPTIterating {
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6416153805979648.ppt", Exception.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt", RuntimeException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt", FileNotFoundException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt", FileNotFoundException.class);
}
public static Stream<Arguments> files() {

View File

@ -19,12 +19,20 @@ package org.apache.poi.hslf.dev;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.apache.poi.EmptyFileException;
import org.junit.jupiter.api.Test;
public class TestPPDrawingTextListing extends BaseTestPPTIterating {
static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt");
}
@Test
void testMain() throws IOException {
// calls System.exit(): PPDrawingTextListing.main(new String[0]);
@ -33,6 +41,17 @@ public class TestPPDrawingTextListing extends BaseTestPPTIterating {
@Override
void runOneFile(File pFile) throws Exception {
PPDrawingTextListing.main(new String[]{pFile.getAbsolutePath()});
try {
PPDrawingTextListing.main(new String[]{pFile.getAbsolutePath()});
} catch (IndexOutOfBoundsException | IOException e) {
if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
throw e;
}
}
// work around one file which works here but not in other tests
if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt")) {
throw new FileNotFoundException();
}
}
}

View File

@ -34,6 +34,8 @@ public class TestPPTXMLDump extends BaseTestPPTIterating {
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5306877435838464.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6032591399288832.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
LOCAL_EXCLUDED.add("ppt_with_png_encrypted.ppt");
}
@Test
@ -52,14 +54,18 @@ public class TestPPTXMLDump extends BaseTestPPTIterating {
void runOneFile(File pFile) throws Exception {
try {
PPTXMLDump.main(new String[]{pFile.getAbsolutePath()});
if (LOCAL_EXCLUDED.contains(pFile.getName())) {
throw new IllegalStateException("Expected failure for file " + pFile + ", but processing did not throw an exception");
}
} catch (IndexOutOfBoundsException | IOException e) {
if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
throw e;
}
}
// work around one file which works here but not in other tests
if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt")) {
// work around two files which works here but not in other tests
if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt") ||
pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt")) {
throw new FileNotFoundException();
}
}

View File

@ -31,6 +31,7 @@ public class TestSlideIdListing extends BaseTestPPTIterating {
static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5306877435838464.ppt");
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
}
@Test
@ -46,7 +47,7 @@ public class TestSlideIdListing extends BaseTestPPTIterating {
void runOneFile(File pFile) throws Exception {
try {
SlideIdListing.main(new String[]{pFile.getAbsolutePath()});
} catch (IllegalArgumentException e) {
} catch (RuntimeException e) {
if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
throw e;
}

View File

@ -38,6 +38,7 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
FAILING.add("cryptoapi-proc2356.ppt");
FAILING.add("41384.ppt");
FAILING.add("bug56240.ppt");
FAILING.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
}
@Test
@ -66,7 +67,7 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
}
} catch (FileNotFoundException e) {
// some old files are not detected correctly
if(!OLD_FILES.contains(pFile.getName())) {
if(!FAILING.contains(pFile.getName()) && !OLD_FILES.contains(pFile.getName())) {
throw e;
}
}

View File

@ -20,12 +20,19 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.apache.poi.EmptyFileException;
import org.apache.poi.hslf.HSLFTestDataSamples;
import org.junit.jupiter.api.Test;
public class TestSlideShowRecordDumper extends BaseTestPPTIterating {
static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
}
@Test
void testMain() throws IOException {
SlideShowRecordDumper.main(new String[] {
@ -47,6 +54,12 @@ public class TestSlideShowRecordDumper extends BaseTestPPTIterating {
@Override
void runOneFile(File pFile) throws Exception {
SlideShowRecordDumper.main(new String[]{pFile.getAbsolutePath()});
try {
SlideShowRecordDumper.main(new String[]{pFile.getAbsolutePath()});
} catch (IllegalStateException e) {
if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
throw e;
}
}
}
}

View File

@ -20,11 +20,18 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.apache.poi.EmptyFileException;
import org.junit.jupiter.api.Test;
public class TestUserEditAndPersistListing extends BaseTestPPTIterating {
static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
static {
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
}
@Test
void testMain() throws IOException {
// calls System.exit(): UserEditAndPersistListing.main(new String[0]);
@ -33,6 +40,12 @@ public class TestUserEditAndPersistListing extends BaseTestPPTIterating {
@Override
void runOneFile(File pFile) throws Exception {
UserEditAndPersistListing.main(new String[]{pFile.getAbsolutePath()});
try {
UserEditAndPersistListing.main(new String[]{pFile.getAbsolutePath()});
} catch (IllegalStateException e) {
if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
throw e;
}
}
}
}

View File

@ -61,7 +61,7 @@ public class AgileEncryptionHeader extends EncryptionHeader {
setFlags(0);
setSizeExtra(0);
setCspName(null);
setBlockSize(keyData.getBlockSize());
setBlockSize(keyData.getBlockSize() == null ? 0 : keyData.getBlockSize());
setChainingMode(keyData.getCipherChaining());

Binary file not shown.