russellbanks / Komac

The Community Manifest Creator for WinGet
GNU General Public License v3.0
205 stars 17 forks source link

[Bug]: Nested nullsoft installer detected as exe #542

Open aaronliu0130 opened 6 months ago

aaronliu0130 commented 6 months ago

Is there an existing issue for this?

What happened?

komac update -i Waterfox.Waterfox --version G6.0.11 --urls https://cdn1.waterfox.net/waterfox/releases/G6.0.11/WINNT_x86_64/Waterfox%20Setup%20G6.0.11.exe -o . Despite the previous manifest having an installer type of nullsoft, komac changed it to exe. In one occasion, it got changed to portable.

russellbanks commented 6 months ago

This is a UPX installer rather than a Nullsoft installer which is why the installer type is exe. The installer type changing to portable is Komac trying to find if it's a generic installer rather than a known one like Nullsoft or Inno. I'll make a change to this so it gets identified as an exe.

russellbanks commented 6 months ago

I'll keep this open and see if there's a way to unpack UPX installers to find out what their underlying installer is.

aaronliu0130 commented 6 months ago

Can it preserve the previous installer type if it's more specific than exe?

a-mnich commented 4 months ago

@russellbanks I also had a case where the installer type was wrongly set to portable.
However based on the current komac implementation I would have expected that the basic installer logic applies here as the original filename contains "setup". I can't seem to figure out why this didn't work here.

grafik

komac update --identifier mitmproxy.mitmproxy --version 10.3.0 --urls https://downloads.mitmproxy.org/10.3.0/mitmproxy-10.3.0-windows-x86_64-installer.exe
russellbanks commented 4 months ago

@russellbanks I also had a case where the installer type was wrongly set to portable. However based on the current komac implementation I would have expected that the basic installer logic applies here as the original filename contains "setup". I can't seem to figure out why this didn't work here.

Thanks for letting me know @a-mnich. I've been looking into this today and the issue appears to be that the StringFileInfo in the file doesn't lie on a 32-bit boundary when it should.

However, PowerShell and others like VirusTotal are still able to get the VSVersionInfo data despite that so I'm now experimenting with yara-x which is an early rewrite of VirusTotal's yara library, which is what they use to analyse malware.

Utesgui commented 4 months ago

@russellbanks here is another case where komac set the InstallerType to portabel instead of nullsoft :) : https://github.com/microsoft/winget-pkgs/pull/153880

russellbanks commented 4 months ago

@a-mnich, I've changed the PE analysis to use yara-x in https://github.com/russellbanks/Komac/commit/8cab8401bec01329b33165e503a447275b03cb15 which resolves the issue with mitmproxy.mitmproxy.

russellbanks commented 4 months ago

@russellbanks here is another case where komac set the InstallerType to portabel instead of nullsoft :) : microsoft/winget-pkgs#153880

@Utesgui, this one is an odd one because the installer isn't obvious that it's a Nullsoft installer. However, VirusTotal identifies as it likely to be Nullsoft. I'll work on the identifying installers when they don't have the usual identifiers for being a Nullsoft installer. For now, I've added a check so if the last InstallerType is not portable but the new one has been identified as portable, use the old installer type instead.

aaronliu0130 commented 4 months ago

Great! Could you perhaps add the same check for the new one being detected as exe?

russellbanks commented 3 hours ago

@russellbanks here is another case where komac set the InstallerType to portabel instead of nullsoft :) : microsoft/winget-pkgs#153880

This is fixed in komac v2.7.0 as this brings much deeper analysis and understanding of Nullsoft installers.