fix: improve PEAnalyzer reliability by migrating to maintained PE/COFF 4J library fork#8245
Conversation
chadlwilson
force-pushed
the
improve-pe-analyzer
branch
from
5cd3b4c to
7f728ea
Compare
| /** | ||
| * Duplicates {@link PE#read(IDataReader)} with added error handling to swallow EOFExceptions to be more lenient | ||
| * for certain file sections. | ||
| * @see PE#read(IDataReader) | ||
| */ | ||
| private static PE read(IDataReader dr, String context) throws IOException { |
| /** | ||
| * Duplicates {@link PE#readDebugRawData(PE, DataEntry, IDataReader)} since it is private. | ||
| * @see PE#readDebugRawData(PE, DataEntry, IDataReader) | ||
| */ | ||
| private static void readDebugRawData(PE pe, DataEntry entry, IDataReader dr) throws IOException { |
chadlwilson
force-pushed
the
improve-pe-analyzer
branch
from
7f728ea to
6695007
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the PEAnalyzer from the unmaintained org.whitesource:pecoff4j library (version 0.0.2.1) to a semi-maintained fork at com.kichik.pecoff4j (version 0.4.1). The primary goal is to fix EOFException errors that occur when scanning certain DLL files, making the analyzer more resilient to partially corrupt or non-standard PE files.
Changes:
- Migrated dependency from org.whitesource:pecoff4j to com.kichik.pecoff4j fork (version 0.4.1)
- Refactored PEParser to use the new library's API with enhanced EOFException handling for lenient parsing
- Fixed resource leak by implementing proper try-with-resources for file streams
- Added test case for jnidispatch.dll to verify parsing of files with partially broken image data
- Fixed logger class reference and improved logging practices in PEAnalyzer
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Updated pecoff4j dependency groupId and version in dependency management |
| core/pom.xml | Updated pecoff4j dependency groupId to match new fork |
| core/src/main/java/org/owasp/dependencycheck/utils/PEParser.java | Simplified parser by delegating to library methods, added EOFException swallowing for lenient parsing, and fixed resource leak with try-with-resources |
| core/src/main/java/org/owasp/dependencycheck/analyzer/PEAnalyzer.java | Updated imports to new library, migrated to new API methods (StringPair iteration, VersionInfo.read), fixed logger class, improved string checks, and updated logging to use parameterized format |
| core/src/test/java/org/owasp/dependencycheck/analyzer/PEAnalyzerTest.java | Removed unused imports and added test for jnidispatch.dll parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chadlwilson
force-pushed
the
improve-pe-analyzer
branch
from
6695007 to
b4ab66f
Compare
Signed-off-by: Chad Wilson <[email protected]>
… experimental ones) Signed-off-by: Chad Wilson <[email protected]>
chadlwilson
force-pushed
the
improve-pe-analyzer
branch
from
b4ab66f to
c873c6b
Compare
| <groupId>com.kichik.pecoff4j</groupId> | ||
| <artifactId>pecoff4j</artifactId> | ||
| <version>0.0.2.1</version> | ||
| <version>0.4.1</version> |
There was a problem hiding this comment.
Whitesource seemed to originally fork from this repo; which imported from sourceforge/CVS anyway, according to https://github.com/whitesource/pecoff4j-maven so this shouldn't be a big concern.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chadlwilson
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chadlwilson
changed the title
Description of Change
Related issues
Scanning problems
CLI arg
Have test cases been added to cover the new functionality?
kinda.
e_sqllite3.dllfrom java.io.EOFException: Expected to read bytes from the stream on e_sqlite3.dll when enableExperimental enabled #8242 (it is too large to commit; but extracts version3.50.4.5. Seems to be no other vendor etc info in the headers that can be found though.Microsoft.VisualStudio.TestPlatform.ObjectModel.dll18.0.1from PE Analyzer throwing errors while scanning .dll files #7876 (fails before, now extracts everything OK)Dapper.dll1.50.4from [ERROR] java.io.EOFException: Expected to read bytes from the stream (in PE Analyzer) #4619 (fails before, now extracts everything OK)