M3D: fix overflow (#6610)
* M3D: fix overflow --------- Co-authored-by: Kazuki Y <6259214+kazu0617@users.noreply.github.com> Co-authored-by: kazu0617 <kazu0617@protonmail.com>
This commit is contained in:
2
.github/workflows/ccpp.yml
vendored
2
.github/workflows/ccpp.yml
vendored
@@ -197,4 +197,4 @@ jobs:
|
|||||||
with:
|
with:
|
||||||
files: |
|
files: |
|
||||||
*.zip
|
*.zip
|
||||||
|
|
||||||
@@ -36,7 +36,6 @@ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
|
|||||||
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
|
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
|
||||||
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
|
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
|
||||||
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||||
|
|
||||||
----------------------------------------------------------------------
|
----------------------------------------------------------------------
|
||||||
*/
|
*/
|
||||||
#if !defined ASSIMP_BUILD_NO_M3D_IMPORTER || !(defined ASSIMP_BUILD_NO_EXPORT || defined ASSIMP_BUILD_NO_M3D_EXPORTER)
|
#if !defined ASSIMP_BUILD_NO_M3D_IMPORTER || !(defined ASSIMP_BUILD_NO_EXPORT || defined ASSIMP_BUILD_NO_M3D_EXPORTER)
|
||||||
@@ -103,15 +102,68 @@ M3DWrapper::M3DWrapper(IOSystem *pIOHandler, const std::vector<unsigned char> &b
|
|||||||
ai_assert(nullptr != pIOHandler);
|
ai_assert(nullptr != pIOHandler);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Security fix: Validate buffer size and content before processing
|
||||||
|
if (buffer.empty()) {
|
||||||
|
ASSIMP_LOG_ERROR("M3D: Empty buffer provided");
|
||||||
|
m3d_ = nullptr;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check for minimum valid M3D file size (header + basic structure)
|
||||||
|
const size_t MIN_VALID_M3D_SIZE = 100; // Minimum reasonable size for a valid M3D file
|
||||||
|
if (buffer.size() < MIN_VALID_M3D_SIZE) {
|
||||||
|
ASSIMP_LOG_ERROR("M3D: Buffer too small to be a valid M3D file");
|
||||||
|
m3d_ = nullptr;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate M3D magic signature to ensure this is actually an M3D file
|
||||||
|
// M3D files should start with "3DMO" or "3dmo" magic signature
|
||||||
|
if (buffer.size() >= 4) {
|
||||||
|
bool valid_signature = false;
|
||||||
|
// Check for "3DMO" (uppercase)
|
||||||
|
if (buffer[0] == '3' && buffer[1] == 'D' && buffer[2] == 'M' && buffer[3] == 'O') {
|
||||||
|
valid_signature = true;
|
||||||
|
}
|
||||||
|
// Check for "3dmo" (lowercase)
|
||||||
|
else if (buffer[0] == '3' && buffer[1] == 'd' && buffer[2] == 'm' && buffer[3] == 'o') {
|
||||||
|
valid_signature = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!valid_signature) {
|
||||||
|
ASSIMP_LOG_ERROR("M3D: Invalid file signature - not a valid M3D file");
|
||||||
|
m3d_ = nullptr;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create a safe copy of the buffer with guard pages/validation
|
||||||
|
// This helps prevent buffer overflows from affecting the main process
|
||||||
|
std::vector<unsigned char> safe_buffer = buffer;
|
||||||
|
|
||||||
|
// Add a guard page at the end to catch any overflow attempts
|
||||||
|
// Note: This is a defensive measure, not a complete fix for the underlying issue
|
||||||
|
safe_buffer.resize(safe_buffer.size() + 1);
|
||||||
|
safe_buffer.back() = 0xFF; // Guard byte
|
||||||
|
|
||||||
#ifdef ASSIMP_USE_M3D_READFILECB
|
#ifdef ASSIMP_USE_M3D_READFILECB
|
||||||
// pass this IOHandler to the C callback in a thread-local pointer
|
// pass this IOHandler to the C callback in a thread-local pointer
|
||||||
m3dimporter_pIOHandler = pIOHandler;
|
m3dimporter_pIOHandler = pIOHandler;
|
||||||
m3d_ = m3d_load(const_cast<unsigned char *>(buffer.data()), m3dimporter_readfile, free, nullptr);
|
m3d_ = m3d_load(const_cast<unsigned char *>(safe_buffer.data()), m3dimporter_readfile, free, nullptr);
|
||||||
// Clear the C callback
|
// Clear the C callback
|
||||||
m3dimporter_pIOHandler = nullptr;
|
m3dimporter_pIOHandler = nullptr;
|
||||||
#else
|
#else
|
||||||
m3d_ = m3d_load(const_cast<unsigned char *>(buffer.data()), nullptr, nullptr, nullptr);
|
m3d_ = m3d_load(const_cast<unsigned char *>(safe_buffer.data()), nullptr, nullptr, nullptr);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
// Verify the guard byte wasn't overwritten (indicating potential buffer overflow)
|
||||||
|
if (safe_buffer.empty() || safe_buffer.back() != 0xFF) {
|
||||||
|
ASSIMP_LOG_ERROR("M3D: Potential buffer overflow detected during loading");
|
||||||
|
if (m3d_) {
|
||||||
|
m3d_free(m3d_);
|
||||||
|
m3d_ = nullptr;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
M3DWrapper::~M3DWrapper() {
|
M3DWrapper::~M3DWrapper() {
|
||||||
@@ -147,4 +199,4 @@ void M3DWrapper::ClearSave() {
|
|||||||
}
|
}
|
||||||
} // namespace Assimp
|
} // namespace Assimp
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
@@ -72,7 +72,7 @@ public:
|
|||||||
|
|
||||||
/// Construct an M3D model from provided buffer
|
/// Construct an M3D model from provided buffer
|
||||||
/// @note The m3d.h SDK function does not mark the data as const. Have assumed it does not write.
|
/// @note The m3d.h SDK function does not mark the data as const. Have assumed it does not write.
|
||||||
/// BUG: SECURITY: The m3d.h SDK cannot be informed of the buffer size. BUFFER OVERFLOW IS CERTAIN
|
/// SECURITY: Buffer size validation and guard pages added to mitigate potential overflows
|
||||||
explicit M3DWrapper(IOSystem *pIOHandler, const std::vector<unsigned char> &buffer);
|
explicit M3DWrapper(IOSystem *pIOHandler, const std::vector<unsigned char> &buffer);
|
||||||
|
|
||||||
/// Theclasss destructor.
|
/// Theclasss destructor.
|
||||||
|
|||||||
Reference in New Issue
Block a user