From 72b1522083b2bb5cca8a71259484010ac285eb49 Mon Sep 17 00:00:00 2001 From: cutealien Date: Fri, 6 May 2022 19:47:38 +0000 Subject: [PATCH] Add IImage::checkDataSizeLimit and make IImage getDataSizeFromFormat return size_t It's to allow image loader to check for sane limits for image sizes. Idea came from this patch from sfan5 for Minetest: https://github.com/minetest/irrlicht/commit/dbd39120e7ed8c0c97e48e2df62347627f3c1d42 Thought solution is a bit different. Image loader checks not yet added (will come soon). Also note that limit is to s32. While u32 might work mostly it will run into some troubles with color converter for now (which maybe could be changes). Also 2GB ought to be enough for anybody, right? git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6386 dfc29bdd-3216-0410-991c-e03cc46cb475 --- changes.txt | 1 + include/IImage.h | 40 +++++++++++++++++++--------- source/Irrlicht/CD3D9Texture.cpp | 4 +-- source/Irrlicht/CImage.cpp | 10 +++---- source/Irrlicht/CImageLoaderDDS.cpp | 4 +-- source/Irrlicht/CImageLoaderPVR.cpp | 4 +-- source/Irrlicht/COpenGLCoreTexture.h | 4 +-- 7 files changed, 41 insertions(+), 26 deletions(-) diff --git a/changes.txt b/changes.txt index d2d0bcad..294bcd6c 100644 --- a/changes.txt +++ b/changes.txt @@ -1,6 +1,7 @@ -------------------------- Changes in 1.9 (not yet released) +- Add IImage::checkDataSizeLimit and make IImage getDataSizeFromFormat return size_t so image loaders can check if sizes are sane. - Fix crash with large jpg files. Based somewhat on a patch in Minetest from sfan5 https://github.com/minetest/irrlicht/commit/594de9915346a87f67cd94e28c7933993efb5d3b - COSOperator::getSystemMemory now returns some value on OSX (thought same for total and available). Thanks @sfan5 for patch https://irrlicht.sourceforge.io/forum/viewtopic.php?f=2&t=52819 and https://github.com/minetest/irrlicht/commit/e469c54f76f6d24f92389b4e8a27b9cce7152888 diff --git a/include/IImage.h b/include/IImage.h index b09d3c26..dc958cc5 100644 --- a/include/IImage.h +++ b/include/IImage.h @@ -84,7 +84,7 @@ public: } //! Returns image data size in bytes - u32 getImageDataSizeInBytes() const + size_t getImageDataSizeInBytes() const { return getDataSizeFromFormat(Format, Size.Width, Size.Height); } @@ -295,7 +295,7 @@ public: } else { - u32 dataSize = 0; + size_t dataSize = 0; u32 width = Size.Width; u32 height = Size.Height; @@ -445,43 +445,57 @@ public: } } - //! calculate image data size in bytes for selected format, width and height. - static u32 getDataSizeFromFormat(ECOLOR_FORMAT format, u32 width, u32 height) + //! You should not create images where the result of getDataSizeFromFormat doesn't pass this function + /** Note that CImage does not yet check for this, but going beyond this limit is not supported well. + Image loaders should check for this. + If you don't have the format yet then checking width*height*bytes_per_pixel is mostly fine, but make + sure to work with size_t so it doesn't clip the result to u32 too early. + \return true when dataSize is small enough that it should be fine. */ + static bool checkDataSizeLimit(size_t dataSize) { - u32 imageSize = 0; + // 2gb for now. Could be we could do more on some platforms, but we still will run into + // problems right now then for example in then color converter (which currently still uses + // s32 for sizes). + return (size_t)(s32)(dataSize) == dataSize; + } + + //! calculate image data size in bytes for selected format, width and height. + static size_t getDataSizeFromFormat(ECOLOR_FORMAT format, u32 width, u32 height) + { + size_t imageSize = 0; switch (format) { case ECF_DXT1: - imageSize = ((width + 3) / 4) * ((height + 3) / 4) * 8; + imageSize = (size_t)((width + 3) / 4) * ((height + 3) / 4) * 8; break; case ECF_DXT2: case ECF_DXT3: case ECF_DXT4: case ECF_DXT5: - imageSize = ((width + 3) / 4) * ((height + 3) / 4) * 16; + imageSize = (size_t)((width + 3) / 4) * ((height + 3) / 4) * 16; break; case ECF_PVRTC_RGB2: case ECF_PVRTC_ARGB2: - imageSize = (core::max_(width, 16) * core::max_(height, 8) * 2 + 7) / 8; + imageSize = ((size_t)core::max_(width, 16) * core::max_(height, 8) * 2 + 7) / 8; break; case ECF_PVRTC_RGB4: case ECF_PVRTC_ARGB4: - imageSize = (core::max_(width, 8) * core::max_(height, 8) * 4 + 7) / 8; + imageSize = ((size_t)core::max_(width, 8) * core::max_(height, 8) * 4 + 7) / 8; break; case ECF_PVRTC2_ARGB2: - imageSize = core::ceil32(width / 8.0f) * core::ceil32(height / 4.0f) * 8; + imageSize = (size_t)core::ceil32(width / 8.0f) * core::ceil32(height / 4.0f) * 8; break; case ECF_PVRTC2_ARGB4: case ECF_ETC1: case ECF_ETC2_RGB: - imageSize = core::ceil32(width / 4.0f) * core::ceil32(height / 4.0f) * 8; + imageSize = (size_t)core::ceil32(width / 4.0f) * core::ceil32(height / 4.0f) * 8; break; case ECF_ETC2_ARGB: - imageSize = core::ceil32(width / 4.0f) * core::ceil32(height / 4.0f) * 16; + imageSize = (size_t)core::ceil32(width / 4.0f) * core::ceil32(height / 4.0f) * 16; break; default: // uncompressed formats - imageSize = getBitsPerPixelFromFormat(format) / 8 * width; + imageSize = (size_t)getBitsPerPixelFromFormat(format) / 8 * width; imageSize *= height; break; } diff --git a/source/Irrlicht/CD3D9Texture.cpp b/source/Irrlicht/CD3D9Texture.cpp index 7b31fcc7..da398a0b 100644 --- a/source/Irrlicht/CD3D9Texture.cpp +++ b/source/Irrlicht/CD3D9Texture.cpp @@ -335,7 +335,7 @@ void CD3D9Texture::regenerateMipMapLevels(void* data, u32 layer) u32 width = Size.Width; u32 height = Size.Height; u8* tmpData = static_cast(data); - u32 dataSize = 0; + size_t dataSize = 0; u32 level = 0; do @@ -714,7 +714,7 @@ void CD3D9Texture::uploadTexture(void* data, u32 mipmapLevel, u32 layer) u32 width = Size.Width >> mipmapLevel; u32 height = Size.Height >> mipmapLevel; - u32 dataSize = IImage::getDataSizeFromFormat(ColorFormat, width, height); + size_t dataSize = IImage::getDataSizeFromFormat(ColorFormat, width, height); HRESULT hr = 0; diff --git a/source/Irrlicht/CImage.cpp b/source/Irrlicht/CImage.cpp index c97f35f5..1f9bbb50 100644 --- a/source/Irrlicht/CImage.cpp +++ b/source/Irrlicht/CImage.cpp @@ -24,8 +24,7 @@ CImage::CImage(ECOLOR_FORMAT format, const core::dimension2d& size, void* d } else { - const u32 dataSize = getDataSizeFromFormat(Format, Size.Width, Size.Height); - + const size_t dataSize = getDataSizeFromFormat(Format, Size.Width, Size.Height); Data = new u8[align_next(dataSize,16)]; memcpy(Data, data, dataSize); DeleteMemory = true; @@ -36,7 +35,8 @@ CImage::CImage(ECOLOR_FORMAT format, const core::dimension2d& size, void* d //! Constructor of empty image CImage::CImage(ECOLOR_FORMAT format, const core::dimension2d& size) : IImage(format, size, true) { - Data = new u8[align_next(getDataSizeFromFormat(Format, Size.Width, Size.Height),16)]; + const size_t dataSize = getDataSizeFromFormat(Format, Size.Width, Size.Height); + Data = new u8[align_next(dataSize,16)]; DeleteMemory = true; } @@ -332,8 +332,8 @@ void CImage::fill(const SColor &color) { u8 rgb[3]; CColorConverter::convert_A8R8G8B8toR8G8B8(&color, 1, rgb); - const u32 size = getImageDataSizeInBytes(); - for (u32 i=0; i CImageLoaderPVR::loadImages(io::IReadFile* file, E_TEXTURE_ core::array mipMapsDataArray; ECOLOR_FORMAT format = ECF_UNKNOWN; - u32 dataSize = 0; + size_t dataSize = 0; file->seek(0); file->read(&header, sizeof(SPVRHeader)); @@ -216,7 +216,7 @@ core::array CImageLoaderPVR::loadImages(io::IReadFile* file, E_TEXTURE_ // read texture dataSize = 0; - long offset = 0; + size_t offset = 0; for (u32 i = 0; i < header.MipMapCount; ++i) { diff --git a/source/Irrlicht/COpenGLCoreTexture.h b/source/Irrlicht/COpenGLCoreTexture.h index 4e354a9e..c431ee9a 100644 --- a/source/Irrlicht/COpenGLCoreTexture.h +++ b/source/Irrlicht/COpenGLCoreTexture.h @@ -401,7 +401,7 @@ public: u32 width = Size.Width; u32 height = Size.Height; u8* tmpData = static_cast(data); - u32 dataSize = 0; + size_t dataSize = 0; u32 level = 0; do @@ -600,7 +600,7 @@ protected: } else { - u32 dataSize = IImage::getDataSizeFromFormat(ColorFormat, width, height); + GLsizei dataSize = (GLsizei)IImage::getDataSizeFromFormat(ColorFormat, width, height); switch (TextureType) {