Use AMSI for archive malware scanning #4910
Conversation
* Remove `#include <pure.h>` and add necessary includes for AMSI in `src/AppInstallerCommonCore/Archive.cpp` * Initialize AMSI, create a session, scan the file, and handle results in `ScanZipFile` function * Add tests for new archive formats in `src/AppInstallerCLITests/Archive.cpp` - Add test cases for 7z, Rar, TarGz, and TarBz2 archive formats - Verify extraction and scanning of these new archive formats
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
If AMSI was a suitable replacement to PureLib, I'd be all for changing it just to have one less external dependency.
But I don't think it is a good replacement based on the documentation. I only see mentions of AMSI supporting scripts or .NET binaries, while PureLib is for scanning ZIP files specifically. For example, it's not clear to me that AMSI would catch a zip bomb.
There was a problem hiding this comment.
This may be me being paranoid but I would be slightly concerned about having a binary checked without knowing what it is. Would be great if you could share how it gas generated so that we can validate that it's that (assuming it's a deterministic process)
| @@ -274,6 +274,21 @@ | |||
| <SubSystem Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">Windows</SubSystem> | |||
| <SubSystem Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Windows</SubSystem> | |||
| </Link> | |||
| <Lib> | |||
| <AdditionalDependencies Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">amsi.lib;%(AdditionalDependencies)</AdditionalDependencies> | |||
There was a problem hiding this comment.
Huh, I wonder why we don't have the other dependencies here. I think they are listed in the projects that produce the final binaries, like WindowsPackageManager.vcxproj. Maybe we could avoid some duplication by having the dependencies at the point they are introduced...
(Not a comment for this PR)
| @@ -18,6 +18,7 @@ | |||
| #include <wincrypt.h> | |||
| #include <deliveryoptimization.h> | |||
| #include <deliveryoptimizationerrors.h> | |||
| #include <amsi.h> | |||
There was a problem hiding this comment.
I'm not sure if we need it in pch.h if it is only used in Archive.cpp. But I do see the TODO comment from before about moving it 🤔
| // TODO: Move include statement to pch.h and resolve build errors | ||
| #pragma warning( push ) | ||
| #pragma warning ( disable : 4189 4244 26451 ) | ||
| #include <pure.h> |
There was a problem hiding this comment.
IF we do decide to go with AMSI instead of PureLib, there are a bunch other places we have to remove it from. We have the source code at src/PureLib/, all the references in AdditionalIncludeDirectories and AdditionalDependencies, and removing the project from the solution. There's also the some housekeeping to do with the cgmanifest and the notice file
| HAMSICONTEXT amsiContext; | ||
| HAMSISESSION amsiSession; |
There was a problem hiding this comment.
These should use one of the WIL magic RAII wrappers to make the destruction easier and ensure it happens. I don't see any specific for AMSI, so it would probably be a wil::unique_any
|
|
||
| constexpr std::string_view s_ZipFile = "TestZip.zip"; | ||
| constexpr std::string_view s_TarGzFile = "TestTarGz.tar.gz"; | ||
| constexpr std::string_view s_Large7zFile = "TestLarge7z.7z"; |
Most antivirus software should be able to detect zip bomb. I'm using ESET and just tried out AMSI with this code: https://gist.github.com/SpecterShell/c2b5507ba9e5c531faead38c7d0b877e And ESET passed me a notification for detecting the zip bomb. |
Replace pure with AMSI for archive malware scanning, to allow archive types other than zip. Added tests for
tgzwitharchiveExtractionMethod tarand a stub for testing a large file. I tested with a 1GB compressed 7z and scanning took slightly under two minutes on a 4 core VM.This was partially an experiment to see how Copilot Workspace handles C++/Windows, no worries if this feature isn't useful.
For more details, open the Copilot Workspace session.
Microsoft Reviewers: Open in CodeFlow