Bug 69667: Handle slightly broken WriteAccessRecord gracefully

It seems some software creates records with invalid length.

If it uses UTF-16LE encoding, we can end up with 109 bytes, 
which is invalid as UTF-16LE always requires an even number of 
bytes.

Therefor we now sanitize the number of bytes we read from the 
record to avoid this issue.

Also improve error message and add tests

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1925419 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2025-05-05 17:23:59 +00:00
parent 22192ce2cc
commit 9e30ffc0de
2 changed files with 55 additions and 2 deletions

View File

@ -108,10 +108,19 @@ public final class WriteAccessRecord extends StandardRecord {
data = IOUtils.safelyAllocate(in.remaining(), STRING_SIZE);
in.readFully(data);
if (UTF16FLAG.isSet(is16BitFlag)) {
byteCnt = Math.min(nChars * 2, data.length);
// the spec only allows up to 109 bytes for the string in this record, but it seems some broken
// software out there will generate invalid records
int min = Math.min(nChars * 2, data.length);
// make sure byteCnt is divisible by 2 as we read UTF-16LE
byteCnt = min - (min % 2);
charset = StandardCharsets.UTF_16LE;
} else {
// the spec only allows up to 109 bytes for the string in this record, but it seems some broken
// software out there will generate invalid records
byteCnt = Math.min(nChars, data.length);
charset = StandardCharsets.ISO_8859_1;
}
}
@ -130,7 +139,8 @@ public final class WriteAccessRecord extends StandardRecord {
boolean is16bit = StringUtil.hasMultibyte(username);
int encodedByteCount = username.length() * (is16bit ? 2 : 1);
if (encodedByteCount > STRING_SIZE) {
throw new IllegalArgumentException("Name is too long: " + username);
throw new IllegalArgumentException("Name is too long, expecting up to " + STRING_SIZE +
" bytes, but had: " + encodedByteCount + " bytes: " + username);
}
field_1_username = username;

View File

@ -93,4 +93,47 @@ final class TestWriteAccessRecord {
confirmRecordEncoding(WriteAccessRecord.sid, expectedEncoding, rec.serialize());
}
@Test
void testUTF16LE() {
byte[] data = HexRead.readFromString(""
+ "5c 00 70 00 1C 00 01 "
+ "44 00 61 00 74 00 61 00 20 00 44 00 79 00 6e 00 "
+ "61 00 6d 00 69 00 63 00 73 00 27 00 20 00 53 00 "
+ "70 00 72 00 65 00 61 00 64 00 42 00 75 00 69 00 "
+ "6c 00 64 00 65 00 72 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00"
);
RecordInputStream in = TestcaseRecordInputStream.create(data);
WriteAccessRecord rec = new WriteAccessRecord(in);
assertEquals("Data Dynamics' SpreadBuilder", rec.getUsername());
}
@Test
void testUTF16LE_wrong_size() {
// "0x51" on position 5 is an incorrect size, as it would require 162 bytes to encode as UTF-16LE
// the spec only allows up to 109 bytes for the string in this record, but it seems some broken
// software out there will generate such a file
byte[] data = HexRead.readFromString(""
+ "5c 00 70 00 51 00 01 "
+ "44 00 61 00 74 00 61 00 20 00 44 00 79 00 6e 00 "
+ "61 00 6d 00 69 00 63 00 73 00 27 00 20 00 53 00 "
+ "70 00 72 00 65 00 61 00 64 00 42 00 75 00 69 00 "
+ "6c 00 64 00 65 00 72 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00 20 00 20 00 20 00 20 00 20 00 "
+ "20 00 20 00 20 00"
);
RecordInputStream in = TestcaseRecordInputStream.create(data);
WriteAccessRecord rec = new WriteAccessRecord(in);
assertEquals("Data Dynamics' SpreadBuilder", rec.getUsername());
}
}