Skip to content

Commit

Permalink
Bug 66425: Avoid a NullPointerException found via oss-fuzz
Browse files Browse the repository at this point in the history 
We try to avoid throwing NullPointerException, but it was possible
to trigger one here with a specially crafted input-file

Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62128

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912199 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@centic9
centic9 committed Sep 8, 2023
1 parent f90415e commit 5cb7683
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 65 deletions.
89 changes: 30 additions & 59 deletions poi-scratchpad/src/main/java/org/apache/poi/hwpf/sprm/TableSprmUncompressor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,44 +30,39 @@ public final class TableSprmUncompressor extends SprmUncompressor {

private static final Logger LOG = LogManager.getLogger(TableSprmUncompressor.class);

public TableSprmUncompressor()
{
public TableSprmUncompressor() {
}

public static TableProperties uncompressTAP( SprmBuffer sprmBuffer )
{
public static TableProperties uncompressTAP( SprmBuffer sprmBuffer ) {
TableProperties tableProperties;

if (sprmBuffer == null) {
throw new IllegalArgumentException("Cannot process TableProperties with an empty SprmBuffer");
}

SprmOperation sprmOperation = sprmBuffer.findSprm( (short) 0xd608 );
if ( sprmOperation != null )
{
if ( sprmOperation != null ) {
byte[] grpprl = sprmOperation.getGrpprl();
int offset = sprmOperation.getGrpprlOffset();
short itcMac = grpprl[offset];
tableProperties = new TableProperties( itcMac );
}
else
{
} else {
LOG.atWarn().log("Some table rows didn't specify number of columns in SPRMs");
tableProperties = new TableProperties( (short) 1 );
}

for ( SprmIterator iterator = sprmBuffer.iterator(); iterator.hasNext(); )
{
for ( SprmIterator iterator = sprmBuffer.iterator(); iterator.hasNext(); ) {
SprmOperation sprm = iterator.next();

/*
* TAPXs are actually PAPXs so we have to make sure we are only
* trying to uncompress the right type of sprm.
*/
if ( sprm.getType() == SprmOperation.TYPE_TAP )
{
try
{
if ( sprm.getType() == SprmOperation.TYPE_TAP ) {
try {
unCompressTAPOperation( tableProperties, sprm );
}
catch ( ArrayIndexOutOfBoundsException ex )
{
catch ( ArrayIndexOutOfBoundsException ex ) {
LOG.atError().withThrowable(ex).log("Unable to apply {}", sprm);
}
}
Expand All @@ -82,29 +77,23 @@ public static TableProperties uncompressTAP( SprmBuffer sprmBuffer )
* @param newTAP The TableProperties object to perform the operation on.
* @param sprm The operation to perform.
*/
static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
{
switch (sprm.getOperation())
{
static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm) {
switch (sprm.getOperation()) {
case 0:
newTAP.setJc ((short) sprm.getOperand());
break;
case 0x01:
{
case 0x01: {
short[] rgdxaCenter = newTAP.getRgdxaCenter ();
short itcMac = newTAP.getItcMac ();
int adjust = sprm.getOperand() - (rgdxaCenter[0] + newTAP.getDxaGapHalf ());
for (int x = 0; x < itcMac; x++)
{
for (int x = 0; x < itcMac; x++) {
rgdxaCenter[x] += (short) adjust;
}
break;
}
case 0x02:
{
case 0x02: {
short[] rgdxaCenter = newTAP.getRgdxaCenter ();
if (rgdxaCenter != null)
{
if (rgdxaCenter != null) {
int adjust = newTAP.getDxaGapHalf () - sprm.getOperand();
rgdxaCenter[0] += (short) adjust;
}
Expand All @@ -117,8 +106,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
case 0x04:
newTAP.setFTableHeader (getFlag (sprm.getOperand()));
break;
case 0x05:
{
case 0x05: {
byte[] buf = sprm.getGrpprl();
int offset = sprm.getGrpprlOffset();
newTAP.setBrcTop(new BorderCode(buf, offset));
Expand All @@ -141,8 +129,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
case 0x07:
newTAP.setDyaRowHeight (sprm.getOperand());
break;
case 0x08:
{
case 0x08: {
byte[] grpprl = sprm.getGrpprl();
int offset = sprm.getGrpprlOffset();
short itcMac = grpprl[offset];
Expand All @@ -154,8 +141,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
newTAP.setRgtc (rgtc);

// get the rgdxaCenters
for (int x = 0; x < itcMac; x++)
{
for (int x = 0; x < itcMac; x++) {
rgdxaCenter[x] = LittleEndian.getShort (grpprl, offset + (1 + (x * 2)));
}

Expand All @@ -165,8 +151,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)

boolean hasTCs = startOfTCs < endOfSprm;

for (int x = 0; x < itcMac; x++)
{
for (int x = 0; x < itcMac; x++) {
// Sometimes, the grpprl does not contain data at every offset. I have no idea why this happens.
if(hasTCs && offset + (1 + ( (itcMac + 1) * 2) + (x * 20)) < grpprl.length)
rgtc[x] = TableCellDescriptor.convertBytesToTC(grpprl,
Expand All @@ -193,26 +178,19 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
// for (int x = varParam[0]; x < varParam[1]; x++)
// {
//
// if ((varParam[2] & 0x08) > 0)
// {
// if ((varParam[2] & 0x08) > 0) {
// short[] brcRight = rgtc[x].getBrcRight ();
// brcRight[0] = LittleEndian.getShort (varParam, 6);
// brcRight[1] = LittleEndian.getShort (varParam, 8);
// }
// else if ((varParam[2] & 0x04) > 0)
// {
// } else if ((varParam[2] & 0x04) > 0) {
// short[] brcBottom = rgtc[x].getBrcBottom ();
// brcBottom[0] = LittleEndian.getShort (varParam, 6);
// brcBottom[1] = LittleEndian.getShort (varParam, 8);
// }
// else if ((varParam[2] & 0x02) > 0)
// {
// } else if ((varParam[2] & 0x02) > 0) {
// short[] brcLeft = rgtc[x].getBrcLeft ();
// brcLeft[0] = LittleEndian.getShort (varParam, 6);
// brcLeft[1] = LittleEndian.getShort (varParam, 8);
// }
// else if ((varParam[2] & 0x01) > 0)
// {
// } else if ((varParam[2] & 0x01) > 0) {
// short[] brcTop = rgtc[x].getBrcTop ();
// brcTop[0] = LittleEndian.getShort (varParam, 6);
// brcTop[1] = LittleEndian.getShort (varParam, 8);
Expand All @@ -221,8 +199,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
// break;
// }
break;
case 0x21:
{
case 0x21: {
int param = sprm.getOperand();
int index = (param & 0xff000000) >> 24;
int count = (param & 0x00ff0000) >> 16;
Expand All @@ -231,15 +208,13 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)

short[] rgdxaCenter = new short[itcMac + count + 1];
TableCellDescriptor[] rgtc = new TableCellDescriptor[itcMac + count];
if (index >= itcMac)
{
if (index >= itcMac) {
index = itcMac;
System.arraycopy(newTAP.getRgdxaCenter(), 0, rgdxaCenter, 0,
itcMac + 1);
System.arraycopy(newTAP.getRgtc(), 0, rgtc, 0, itcMac);
}
else
{
else {
//copy rgdxaCenter
System.arraycopy(newTAP.getRgdxaCenter(), 0, rgdxaCenter, 0,
index + 1);
Expand All @@ -251,8 +226,7 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
itcMac - index);
}

for (int x = index; x < index + count; x++)
{
for (int x = index; x < index + count; x++) {
rgtc[x] = new TableCellDescriptor();
rgdxaCenter[x] = (short)(rgdxaCenter[x - 1] + width);
}
Expand Down Expand Up @@ -311,7 +285,4 @@ static void unCompressTAPOperation (TableProperties newTAP, SprmOperation sprm)
break;
}
}



}
8 changes: 3 additions & 5 deletions poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToConverterSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

public class TestWordToConverterSuite
{
public class TestWordToConverterSuite {
/**
* YK: a quick hack to exclude failing documents from the suite.
*/
Expand All @@ -60,7 +59,8 @@ public class TestWordToConverterSuite
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5418937293340672.doc",
"TestHPSFWritingFunctionality.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-4947285593948160.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5440721166139392.doc"
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5440721166139392.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5050208641482752.doc"
);

public static Stream<Arguments> files() {
Expand Down Expand Up @@ -139,6 +139,4 @@ void testText(File child) throws Exception {
// no exceptions
assertNotNull(stringWriter.toString());
}


}
3 changes: 2 additions & 1 deletion poi-scratchpad/src/test/java/org/apache/poi/hwpf/converter/TestWordToTextConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public class TestWordToTextConverter {
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5418937293340672.doc",
"TestHPSFWritingFunctionality.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-4947285593948160.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5440721166139392.doc"
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5440721166139392.doc",
"clusterfuzz-testcase-minimized-POIHWPFFuzzer-5050208641482752.doc"
);

/**
Expand Down
Binary file added test-data/document/clusterfuzz-testcase-minimized-POIHWPFFuzzer-5050208641482752.doc
View file
Binary file not shown.
Binary file modified test-data/spreadsheet/stress.xls
View file
Binary file not shown.

0 comments on commit 5cb7683

Please sign in to comment.