From 9e30ffc0dea566b561dd53acb1906cfc7f752331 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Mon, 5 May 2025 17:23:59 +0000 Subject: [PATCH] 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 --- .../poi/hssf/record/WriteAccessRecord.java | 14 +++++- .../hssf/record/TestWriteAccessRecord.java | 43 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/poi/src/main/java/org/apache/poi/hssf/record/WriteAccessRecord.java b/poi/src/main/java/org/apache/poi/hssf/record/WriteAccessRecord.java index 88f8affd2d..712693554c 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/WriteAccessRecord.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/WriteAccessRecord.java @@ -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; diff --git a/poi/src/test/java/org/apache/poi/hssf/record/TestWriteAccessRecord.java b/poi/src/test/java/org/apache/poi/hssf/record/TestWriteAccessRecord.java index ee3cfec38b..8286f442c0 100644 --- a/poi/src/test/java/org/apache/poi/hssf/record/TestWriteAccessRecord.java +++ b/poi/src/test/java/org/apache/poi/hssf/record/TestWriteAccessRecord.java @@ -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()); + } }