Bug 58069 - Biff8RC4 xorShort returns wrong value for unsigned shorts
Summary: Biff8RC4 xorShort returns wrong value for unsigned shorts
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.13-dev
Hardware: All All
: P2 regression (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2015-06-22 18:28 UTC by Peter Royal
Modified: 2015-06-23 23:07 UTC (History)
0 users

Patch for Biff8DecryptingStream (2.82 KB, patch)
2015-06-22 22:54 UTC, Andreas Beeker
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Royal 2015-06-22 18:28:34 UTC
The change to Biff8RC4 to use a ByteBuffer broke xorShort for unsigned short values.

I noticed this when the loading of a protected XLS file that contained a org.apache.poi.hssf.record.pivottable.ViewFieldsRecord , which uses 0xFFFF as a "not present" value.

3.11+ return -1 rather than 0xFFFF causing the loading of this workbook to fail.

The following patch fixes the loading of my file (and doesn't cause anything else to fail going by 'ant test' return code)

        public int xorShort(int rawVal) {
-           _buffer.putShort(0, (short)rawVal);
+           _buffer.putShort(0, (short) (rawVal & 0xffff));
            xor(_buffer.array(), 0, 2);
-               return _buffer.getShort(0);
+               return _buffer.getShort(0) & 0xffff;
Comment 1 Andreas Beeker 2015-06-22 20:11:29 UTC
Instead of turning a short into an unsigned short on the last line, we should provide a writeUShort method in LittleEndianOutput and change the serialize method in ViewFieldsRecord.

If you like, you can provide your test file, so we can validate it as kind of integration test.
Otherwise I will create a simple test case which test reading and writing into a rc4 stream with various datatypes ...

Comment 2 Peter Royal 2015-06-22 20:43:02 UTC
Unfortunately I can't provide my input file since it is from a third party vendor I am trying to interop with.

I experienced the failure in loading the ViewFieldsRecord, via LittleEndianInput.readUShort
Comment 3 Andreas Beeker 2015-06-22 22:54:59 UTC
Created attachment 32850 [details]
Patch for Biff8DecryptingStream

You are right regarding the reading, readUShort - first we haven't implemented writing for RC4-HSSF yet and I guess even when writing a broader datatype in a smaller, the sign
doesn't matter, e.g. 200 (int) = 0x000000C8 becomes -56 (byte) and needs to be converted
back to unsigned when it's read ...

Is the attached patch for Biff8DecryptingStream working for you?

Comment 4 Peter Royal 2015-06-23 13:16:12 UTC
Your patch fixes loading of my file.

Comment 5 Andreas Beeker 2015-06-23 23:07:35 UTC
Fixed with r1687146

Thank you for bringing this to our attention and testing the patch.