Bug: Reachable assertion (nullptr) error in `D3MFOpcPackage`

Describe the bug

A corrupt .3mf kills the process instead of failing the import. When the root model part cannot be extracted from the ZIP container, the 3MF loader aborts on an assertion rather than returning an error.

D3MFOpcPackage's constructor already handles this case correctly with a throw DeadlyImportError on the line below, but the assertion fires first and calls abort() before the throw is reached.

code/AssetLib/3MF/D3MFOpcPackage.cpp:

162	            mRootPath = rootFile;
163	            mRootStream = mZipArchive->Open(rootFile.c_str());
164	            ai_assert(mRootStream != nullptr);
165	            if (nullptr == mRootStream) {
166	                throw DeadlyImportError("Cannot open root-file in archive : " + rootFile);
167	            }

ai_assert failure in /src/assimp/code/AssetLib/3MF/D3MFOpcPackage.cpp(164): mRootStream != nullptr
==14==ERROR: AddressSanitizer: ABRT on unknown address 0x00000000000e
SCARINESS: 10 (signal)
    #2 in Assimp::defaultAiAssertHandler(char const*, char const*, int) /src/assimp/code/Common/AssertHandler.cpp:53:5
    #3 in Assimp::D3MF::D3MFOpcPackage::D3MFOpcPackage(...) /src/assimp/code/AssetLib/3MF/D3MFOpcPackage.cpp:164:13
    #4 in Assimp::D3MFImporter::InternReadFile(...) /src/assimp/code/AssetLib/3MF/D3MFImporter.cpp:106:20
    #5 in Assimp::BaseImporter::ReadFile(...) /src/assimp/code/Common/BaseImporter.cpp:131:9
    #6 in Assimp::Importer::ReadFile(char const*, unsigned int) /src/assimp/code/Common/Importer.cpp:709:30
    #7 in Assimp::Importer::ReadFileFromMemory(...) /src/assimp/code/Common/Importer.cpp:507:9

The assertion is compiled in for any build that does not define NDEBUG. ai_assert is keyed on ASSIMP_BUILD_DEBUG (include/assimp/ai_assert.h:66-72), and include/assimp/defs.h:257-259 defines that whenever NDEBUG is absent:

257	#if defined(_DEBUG) || !defined(NDEBUG)
258	#  define ASSIMP_BUILD_DEBUG
259	#endif

OSS-Fuzz's base-builder sets no NDEBUG, and projects/assimp/build.sh sets no CMAKE_BUILD_TYPE, so every OSS-Fuzz build of assimp has this assertion enabled, as does any debug build a downstream consumer ships.

To Reproduce

Steps to reproduce the behavior:

1. Get OSS-Fuzz and build the assimp fuzz targets with ASan:

export DOCKER_DEFAULT_PLATFORM=linux/amd64 # if on mac
git clone https://github.com/google/oss-fuzz.git
cd oss-fuzz
python3 infra/helper.py build_image assimp
python3 infra/helper.py build_fuzzers --sanitizer address assimp

1. Take the two attached files, minimal_open_fails.3mf and crash-0002a68bd1ac4c289050394c189aaac8a58c61dd. (Reproduce.zip)

2. Run either of them through the 3MF target:

python3 infra/helper.py reproduce assimp assimp_fuzzer_3mf minimal_open_fails.3mf
python3 infra/helper.py reproduce assimp assimp_fuzzer_3mf crash-0002a68bd1ac4c289050394c189aaac8a58c61dd

1. See the ai_assert failure line quoted above, followed by AddressSanitizer: ABRT.

Both files produce the same DEDUP_TOKEN: raise--abort--Assimp::defaultAiAssertHandler(char const*, char const*, int).

The generic assimp_fuzzer target reproduces both files as well, since it auto-detects the 3MF format.

Expected behavior

Importer::ReadFileFromMemory should return nullptr and Importer::GetErrorString should report Cannot open root-file in archive, which is what the throw DeadlyImportError on line 166 already does.

This is also what already happens today in a build with NDEBUG set, where the assertion is compiled out and the nullptr check below it handles the input correctly.

Platform (please complete the following information):

• OS: OSS-Fuzz at commit a4df12d70b5420567d893b3f53e5818a74df5db7, default linux/amd64 containers
• Browser: not applicable
• Version: assimp master at commit 21bea12f1278090c09a94c39ca090d9fb458ccd9, which reports itself as 6.0.5

Additional context

Suggested fix, delete the assertion and let the existing throw do its job:

             mRootPath = rootFile;
             mRootStream = mZipArchive->Open(rootFile.c_str());
-            ai_assert(mRootStream != nullptr);
             if (nullptr == mRootStream) {
                 throw DeadlyImportError("Cannot open root-file in archive : " + rootFile);
             }

An assertion cannot serve as input validation here, since NDEBUG removes it exactly in the builds that face untrusted files. The nullptr check below it is the real guard and it already works.

With that change, both attached files stop crashing.

Once the assertion is gone, ASan reports a leak of mZipArchive on the throwing path. That is not a regression from this patch, it is pre-existing and is what PR https://github.com/assimp/assimp/pull/6803 fixes, so the two changes complement each other. Note that #6803 rewrites this constructor but keeps this assertion. We built that PR and both attached files still abort, so it does not cover this on its own.

Found by the CISPA Fandango-Team while triaging OSS-Fuzz findings for assimp.