Bug 62625

Summary: VBAMacroReader in 4.0.0-SNAPSHOT, unexpected reserved byte
Product: POI Reporter: Tim Allison <tallison>
Component: POI OverallAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 4.0.x-dev   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: triggering document

Description Tim Allison 2018-08-14 19:00:21 UTC
Created attachment 36092 [details]
triggering document

New exception in 4.0.0-SNAPSHOT.


Caused by: java.io.IOException: Expected 3eafter name before Unicode name, but found: d
	at org.apache.poi.poifs.macros.VBAMacroReader.readStringPair(VBAMacroReader.java:555)
	at org.apache.poi.poifs.macros.VBAMacroReader.processDirStream(VBAMacroReader.java:488)
Comment 1 Tim Allison 2018-10-30 01:26:58 UTC
See decalage's note: https://github.com/decalage2/oletools/blob/master/oletools/olevba.py#L1516

# According to [MS-OVBA] 2.3.4.2.2.2 REFERENCENAME Record:
# "Reserved (2 bytes): MUST be 0x003E. MUST be ignored."
# So let's ignore it, otherwise it crashes on some files (issue #132)
# PR #135 by @c1fe:
# contrary to the specification I think that the unicode name
# is optional. if reference_reserved is not 0x003E I think it
# is actually the start of another REFERENCE record
# at least when projectsyskind_syskind == 0x02 (Macintosh)

Sometimes REFERENCENAME only has an ascii part, contrary to the spec.
Comment 2 Tim Allison 2018-10-30 13:28:00 UTC
r1845238
Comment 3 Tim Allison 2018-10-30 14:12:59 UTC
To clarify, this wasn't a regression.  This was a bug that became apparent with better exception handling.
Comment 4 PJ Fanning 2018-10-30 23:24:42 UTC
The recent code changes seem to have introduced a number of new warnings in https://lgtm.com/projects/g/apache/poi/alerts/?mode=list and in FindBugs output (see the spike in https://builds.apache.org/view/P/view/POI/job/POI-DSL-1.8/)
Comment 5 Tim Allison 2018-10-31 00:07:34 UTC
Doh. Fixing now.  Thank you.
Comment 6 Tim Allison 2018-10-31 00:48:59 UTC
One dodgy loop fixed.  Added other sanity checks.  Thank you, PJ!
Comment 7 Tim Allison 2018-10-31 01:01:52 UTC
And fix made to ensure closing of streams from lgtm.  Thank you, again, PJ.