Skip to content

Conversation

jcrussell
Copy link

Searches for "Nullsoft" in the manifest to avoid false positives. Possibly too strict.

Fixes #1249

Searches for "Nullsoft" in the manifest to avoid false positives.
Possibly too strict.

Fixes onekey-sec#1249
@qkaiser qkaiser self-assigned this Sep 4, 2025
@qkaiser qkaiser self-requested a review September 4, 2025 07:45
@qkaiser qkaiser added enhancement New feature or request format:executable python Pull requests that update Python code labels Sep 4, 2025
Comment on lines 81 to 82
if not self.is_nsis(binary):
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be moved to a custom extractor, say PEExtractor so that if there is a PE file chunk in a file it will be carved out.

In the PEExtractor you can check if it's NSIS and do this if it is:

Command("7z", "x", "-y", "{inpath}", "-o{outdir}").extract(inpath, outdir)

If it's not NSIS, do nothing. Unless you can think of anything to do with "normal" PE ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds good, I update the PR later in the week (sorry for the delay).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np, take your time :)

"""
Test if binary appears to be a Nullsoft Installer self-extracting archive
TODO: this series of tests is possibly too strict
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I leave that call to you. I don't know much about NullSoft.

Comment on lines 65 to 71
if not binary.has_resources:
return False

if not binary.resources_manager.has_manifest:
return False

return "Nullsoft" in binary.resources_manager.manifest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be simplified to:

Suggested change
if not binary.has_resources:
return False
if not binary.resources_manager.has_manifest:
return False
return "Nullsoft" in binary.resources_manager.manifest
return binary.has_resources and binary.resources_manager.has_manifest and "Nullsoft" in binary.resources_manager.manifest

Comment on lines +30 to +31
// MZ header
4d 5a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check for the "PE" signature (50 45 00 00) too ?

@qkaiser
Copy link
Contributor

qkaiser commented Sep 4, 2025

@jcrussell you should also create integration tests to check that the handler works as expected.

You have to create the following directories:

  • unblob/tests/integration/executable/pe/__input__
  • unblob/tests/integration/executable/pe/__output

I would put the following in the input directory:

  • a normal PE file
  • a normal PE file with prefix and suffix padding
  • a nullsoft PE file
  • a nullsoft PE file with prefix and suffix padding

To generate the output directory content, run the following:

find unblob/tests/integration/executable/pe/__input__ -type f -exec unblob -f -k -e unblob/tests/integration/executable/pe/__output__ {} \;

@qkaiser
Copy link
Contributor

qkaiser commented Sep 29, 2025

@jcrussell any update on this ? do you need assistance ?

Create a PEExtractor to handle the logic for extracting from PEs.
Running on non-NSIS PE released by Nullsoft was causing the extractor to
run when we actually don't want it to.
@jcrussell
Copy link
Author

@jcrussell any update on this ? do you need assistance ?

@qkaiser: I believe the code is close to final. Do you mind adding the integration test data? It is easier for me to release code than data. Here's what I have been testing with:

Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request format:executable python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support NSIS Installers
2 participants