From 278e708f2534f1a4ed32c6e64406a73821908fcb Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Wed, 25 Jan 2023 23:25:26 -0800 Subject: [PATCH] ConvertToSinglePlane memory overrwrite bug fix (#307) --- DDSTextureLoader/DDSTextureLoader11.cpp | 38 +++++++++++++++++ DDSTextureLoader/DDSTextureLoader12.cpp | 50 +++++++++++++++++++++++ DirectXTex/DirectXTexConvert.cpp | 12 ++++++ DirectXTex/DirectXTexD3D11.cpp | 48 +++++++++++++++++++++- DirectXTex/DirectXTexD3D12.cpp | 54 ++++++++++++++++++++++++- DirectXTex/DirectXTexDDS.cpp | 5 ++- DirectXTex/DirectXTexImage.cpp | 33 ++++++++------- DirectXTex/DirectXTexP.h | 2 +- DirectXTex/DirectXTexUtil.cpp | 24 +++++++++++ 9 files changed, 245 insertions(+), 21 deletions(-) diff --git a/DDSTextureLoader/DDSTextureLoader11.cpp b/DDSTextureLoader/DDSTextureLoader11.cpp index 7878654..5bb7462 100644 --- a/DDSTextureLoader/DDSTextureLoader11.cpp +++ b/DDSTextureLoader/DDSTextureLoader11.cpp @@ -552,12 +552,22 @@ namespace case DXGI_FORMAT_NV12: case DXGI_FORMAT_420_OPAQUE: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } planar = true; bpe = 2; break; case DXGI_FORMAT_P010: case DXGI_FORMAT_P016: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } planar = true; bpe = 4; break; @@ -1353,6 +1363,34 @@ namespace switch (d3d10ext->dxgiFormat) { + case DXGI_FORMAT_NV12: + case DXGI_FORMAT_P010: + case DXGI_FORMAT_P016: + case DXGI_FORMAT_420_OPAQUE: + if ((d3d10ext->resourceDimension != D3D11_RESOURCE_DIMENSION_TEXTURE2D) + || (width % 2) != 0 || (height % 2) != 0) + { + return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED); + } + break; + + case DXGI_FORMAT_YUY2: + case DXGI_FORMAT_Y210: + case DXGI_FORMAT_Y216: + case DXGI_FORMAT_P208: + if ((width % 2) != 0) + { + return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED); + } + break; + + case DXGI_FORMAT_NV11: + if ((width % 4) != 0) + { + return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED); + } + break; + case DXGI_FORMAT_AI44: case DXGI_FORMAT_IA44: case DXGI_FORMAT_P8: diff --git a/DDSTextureLoader/DDSTextureLoader12.cpp b/DDSTextureLoader/DDSTextureLoader12.cpp index 81caacc..f613199 100644 --- a/DDSTextureLoader/DDSTextureLoader12.cpp +++ b/DDSTextureLoader/DDSTextureLoader12.cpp @@ -631,6 +631,15 @@ namespace case DXGI_FORMAT_NV12: case DXGI_FORMAT_420_OPAQUE: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } + planar = true; + bpe = 2; + break; + case DXGI_FORMAT_P208: planar = true; bpe = 2; @@ -638,6 +647,11 @@ namespace case DXGI_FORMAT_P010: case DXGI_FORMAT_P016: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } planar = true; bpe = 4; break; @@ -1325,12 +1339,48 @@ namespace switch (d3d10ext->dxgiFormat) { + case DXGI_FORMAT_NV12: + case DXGI_FORMAT_P010: + case DXGI_FORMAT_P016: + case DXGI_FORMAT_420_OPAQUE: + if ((d3d10ext->resourceDimension != D3D12_RESOURCE_DIMENSION_TEXTURE2D) + || (width % 2) != 0 || (height % 2) != 0) + { + return HRESULT_E_NOT_SUPPORTED; + } + break; + + case DXGI_FORMAT_YUY2: + case DXGI_FORMAT_Y210: + case DXGI_FORMAT_Y216: + case DXGI_FORMAT_P208: + if ((width % 2) != 0) + { + return HRESULT_E_NOT_SUPPORTED; + } + break; + + case DXGI_FORMAT_NV11: + if ((width % 4) != 0) + { + return HRESULT_E_NOT_SUPPORTED; + } + break; + case DXGI_FORMAT_AI44: case DXGI_FORMAT_IA44: case DXGI_FORMAT_P8: case DXGI_FORMAT_A8P8: return HRESULT_E_NOT_SUPPORTED; + case DXGI_FORMAT_V208: + if ((d3d10ext->resourceDimension != D3D12_RESOURCE_DIMENSION_TEXTURE2D) + || (height % 2) != 0) + { + return HRESULT_E_NOT_SUPPORTED; + } + break; + default: if (BitsPerPixel(d3d10ext->dxgiFormat) == 0) { diff --git a/DirectXTex/DirectXTexConvert.cpp b/DirectXTex/DirectXTexConvert.cpp index bbcc704..1625bef 100644 --- a/DirectXTex/DirectXTexConvert.cpp +++ b/DirectXTex/DirectXTexConvert.cpp @@ -4799,21 +4799,33 @@ namespace { case DXGI_FORMAT_NV12: assert(destImage.format == DXGI_FORMAT_YUY2); + if ((srcImage.width % 2) != 0 || (srcImage.height % 2) != 0) + return E_INVALIDARG; + CONVERT_420_TO_422(uint8_t, XMUBYTEN4); return S_OK; case DXGI_FORMAT_P010: assert(destImage.format == DXGI_FORMAT_Y210); + if ((srcImage.width % 2) != 0 || (srcImage.height % 2) != 0) + return E_INVALIDARG; + CONVERT_420_TO_422(uint16_t, XMUSHORTN4); return S_OK; case DXGI_FORMAT_P016: assert(destImage.format == DXGI_FORMAT_Y216); + if ((srcImage.width % 2) != 0 || (srcImage.height % 2) != 0) + return E_INVALIDARG; + CONVERT_420_TO_422(uint16_t, XMUSHORTN4); return S_OK; case DXGI_FORMAT_NV11: assert(destImage.format == DXGI_FORMAT_YUY2); + if ((srcImage.width % 4) != 0) + return E_INVALIDARG; + // Convert 4:1:1 to 4:2:2 { const size_t rowPitch = srcImage.rowPitch; diff --git a/DirectXTex/DirectXTexD3D11.cpp b/DirectXTex/DirectXTexD3D11.cpp index 555e1ce..ebfe674 100644 --- a/DirectXTex/DirectXTexD3D11.cpp +++ b/DirectXTex/DirectXTexD3D11.cpp @@ -219,6 +219,9 @@ bool DirectX::IsSupportedTexture( if (!IsValid(fmt)) return false; + const size_t iWidth = metadata.width; + const size_t iHeight = metadata.height; + switch (fmt) { case DXGI_FORMAT_BC4_TYPELESS: @@ -241,6 +244,49 @@ bool DirectX::IsSupportedTexture( return false; break; + case DXGI_FORMAT_NV12: + case DXGI_FORMAT_P010: + case DXGI_FORMAT_P016: + case DXGI_FORMAT_420_OPAQUE: + if ((metadata.dimension != TEX_DIMENSION_TEXTURE2D) + || (iWidth % 2) != 0 || (iHeight % 2) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_YUY2: + case DXGI_FORMAT_Y210: + case DXGI_FORMAT_Y216: + case WIN10_DXGI_FORMAT_P208: + if ((iWidth % 2) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_NV11: + if ((iWidth % 4) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_AI44: + case DXGI_FORMAT_IA44: + case DXGI_FORMAT_P8: + case DXGI_FORMAT_A8P8: + // Legacy video stream formats are not supported by Direct3D. + return false; + + case WIN10_DXGI_FORMAT_V208: + if ((metadata.dimension != TEX_DIMENSION_TEXTURE2D) + || (iHeight % 2) != 0) + { + return false; + } + break; + default: break; } @@ -251,8 +297,6 @@ bool DirectX::IsSupportedTexture( // Validate array size, dimension, and width/height const size_t arraySize = metadata.arraySize; - const size_t iWidth = metadata.width; - const size_t iHeight = metadata.height; const size_t iDepth = metadata.depth; // Most cases are known apriori based on feature level, but we use this for robustness to handle the few optional cases diff --git a/DirectXTex/DirectXTexD3D12.cpp b/DirectXTex/DirectXTexD3D12.cpp index 9256193..da18489 100644 --- a/DirectXTex/DirectXTexD3D12.cpp +++ b/DirectXTex/DirectXTexD3D12.cpp @@ -335,14 +335,64 @@ bool DirectX::IsSupportedTexture( if (!IsValid(fmt)) return false; + const size_t iWidth = metadata.width; + const size_t iHeight = metadata.height; + + switch (fmt) + { + case DXGI_FORMAT_NV12: + case DXGI_FORMAT_P010: + case DXGI_FORMAT_P016: + case DXGI_FORMAT_420_OPAQUE: + if ((metadata.dimension != TEX_DIMENSION_TEXTURE2D) + || (iWidth % 2) != 0 || (iHeight % 2) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_YUY2: + case DXGI_FORMAT_Y210: + case DXGI_FORMAT_Y216: + case WIN10_DXGI_FORMAT_P208: + if ((iWidth % 2) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_NV11: + if ((iWidth % 4) != 0) + { + return false; + } + break; + + case DXGI_FORMAT_AI44: + case DXGI_FORMAT_IA44: + case DXGI_FORMAT_P8: + case DXGI_FORMAT_A8P8: + // Legacy video stream formats are not supported by Direct3D. + return false; + + case WIN10_DXGI_FORMAT_V208: + if ((metadata.dimension != TEX_DIMENSION_TEXTURE2D) + || (iHeight % 2) != 0) + { + return false; + } + break; + + default: + break; + } + // Validate miplevel count if (metadata.mipLevels > D3D12_REQ_MIP_LEVELS) return false; // Validate array size, dimension, and width/height const size_t arraySize = metadata.arraySize; - const size_t iWidth = metadata.width; - const size_t iHeight = metadata.height; const size_t iDepth = metadata.depth; // Most cases are known apriori based on feature level, but we use this for robustness to handle the few optional cases diff --git a/DirectXTex/DirectXTexDDS.cpp b/DirectXTex/DirectXTexDDS.cpp index ebbe283..a3888f3 100644 --- a/DirectXTex/DirectXTexDDS.cpp +++ b/DirectXTex/DirectXTexDDS.cpp @@ -1256,8 +1256,9 @@ namespace } size_t pixelSize, nimages; - if (!DetermineImageArray(metadata, cpFlags, nimages, pixelSize)) - return HRESULT_E_ARITHMETIC_OVERFLOW; + HRESULT hr = DetermineImageArray(metadata, cpFlags, nimages, pixelSize); + if (FAILED(hr)) + return hr; if ((nimages == 0) || (nimages != image.GetImageCount())) { diff --git a/DirectXTex/DirectXTexImage.cpp b/DirectXTex/DirectXTexImage.cpp index 4083a2c..ead552c 100644 --- a/DirectXTex/DirectXTexImage.cpp +++ b/DirectXTex/DirectXTexImage.cpp @@ -31,7 +31,7 @@ namespace // Determines number of image array entries and pixel size //------------------------------------------------------------------------------------- _Use_decl_annotations_ -bool DirectX::Internal::DetermineImageArray( +HRESULT DirectX::Internal::DetermineImageArray( const TexMetadata& metadata, CP_FLAGS cpFlags, size_t& nImages, @@ -56,10 +56,11 @@ bool DirectX::Internal::DetermineImageArray( for (size_t level = 0; level < metadata.mipLevels; ++level) { size_t rowPitch, slicePitch; - if (FAILED(ComputePitch(metadata.format, w, h, rowPitch, slicePitch, cpFlags))) + HRESULT hr = ComputePitch(metadata.format, w, h, rowPitch, slicePitch, cpFlags); + if (FAILED(hr)) { nImages = pixelSize = 0; - return false; + return hr; } totalPixelSize += uint64_t(slicePitch); @@ -83,10 +84,11 @@ bool DirectX::Internal::DetermineImageArray( for (size_t level = 0; level < metadata.mipLevels; ++level) { size_t rowPitch, slicePitch; - if (FAILED(ComputePitch(metadata.format, w, h, rowPitch, slicePitch, cpFlags))) + HRESULT hr = ComputePitch(metadata.format, w, h, rowPitch, slicePitch, cpFlags); + if (FAILED(hr)) { nImages = pixelSize = 0; - return false; + return hr; } for (size_t slice = 0; slice < d; ++slice) @@ -109,7 +111,7 @@ bool DirectX::Internal::DetermineImageArray( default: nImages = pixelSize = 0; - return false; + return E_INVALIDARG; } #if defined(_M_IX86) || defined(_M_ARM) || defined(_M_HYBRID_X86_ARM64) @@ -117,7 +119,7 @@ bool DirectX::Internal::DetermineImageArray( if (totalPixelSize > UINT32_MAX) { nImages = pixelSize = 0; - return false; + return HRESULT_E_ARITHMETIC_OVERFLOW; } #else static_assert(sizeof(size_t) == 8, "Not a 64-bit platform!"); @@ -126,7 +128,7 @@ bool DirectX::Internal::DetermineImageArray( nImages = nimages; pixelSize = static_cast(totalPixelSize); - return true; + return S_OK; } @@ -348,8 +350,9 @@ HRESULT ScratchImage::Initialize(const TexMetadata& mdata, CP_FLAGS flags) noexc m_metadata.dimension = mdata.dimension; size_t pixelSize, nimages; - if (!DetermineImageArray(m_metadata, flags, nimages, pixelSize)) - return HRESULT_E_ARITHMETIC_OVERFLOW; + HRESULT hr = DetermineImageArray(m_metadata, flags, nimages, pixelSize); + if (FAILED(hr)) + return hr; m_image = new (std::nothrow) Image[nimages]; if (!m_image) @@ -415,8 +418,9 @@ HRESULT ScratchImage::Initialize2D(DXGI_FORMAT fmt, size_t width, size_t height, m_metadata.dimension = TEX_DIMENSION_TEXTURE2D; size_t pixelSize, nimages; - if (!DetermineImageArray(m_metadata, flags, nimages, pixelSize)) - return HRESULT_E_ARITHMETIC_OVERFLOW; + HRESULT hr = DetermineImageArray(m_metadata, flags, nimages, pixelSize); + if (FAILED(hr)) + return hr; m_image = new (std::nothrow) Image[nimages]; if (!m_image) @@ -466,8 +470,9 @@ HRESULT ScratchImage::Initialize3D(DXGI_FORMAT fmt, size_t width, size_t height, m_metadata.dimension = TEX_DIMENSION_TEXTURE3D; size_t pixelSize, nimages; - if (!DetermineImageArray(m_metadata, flags, nimages, pixelSize)) - return HRESULT_E_ARITHMETIC_OVERFLOW; + HRESULT hr = DetermineImageArray(m_metadata, flags, nimages, pixelSize); + if (FAILED(hr)) + return hr; m_image = new (std::nothrow) Image[nimages]; if (!m_image) diff --git a/DirectXTex/DirectXTexP.h b/DirectXTex/DirectXTexP.h index 9673713..461d8a2 100644 --- a/DirectXTex/DirectXTexP.h +++ b/DirectXTex/DirectXTexP.h @@ -310,7 +310,7 @@ namespace DirectX //--------------------------------------------------------------------------------- // Image helper functions - _Success_(return) bool __cdecl DetermineImageArray( + HRESULT __cdecl DetermineImageArray( _In_ const TexMetadata& metadata, _In_ CP_FLAGS cpFlags, _Out_ size_t& nImages, _Out_ size_t& pixelSize) noexcept; diff --git a/DirectXTex/DirectXTexUtil.cpp b/DirectXTex/DirectXTexUtil.cpp index ea24728..3133c07 100644 --- a/DirectXTex/DirectXTexUtil.cpp +++ b/DirectXTex/DirectXTexUtil.cpp @@ -973,6 +973,11 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height, case DXGI_FORMAT_NV12: case DXGI_FORMAT_420_OPAQUE: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } assert(IsPlanar(fmt)); pitch = ((uint64_t(width) + 1u) >> 1) * 2u; slice = pitch * (uint64_t(height) + ((uint64_t(height) + 1u) >> 1)); @@ -980,6 +985,20 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height, case DXGI_FORMAT_P010: case DXGI_FORMAT_P016: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } + + #if (__cplusplus >= 201703L) + [[fallthrough]]; + #elif defined(__clang__) + [[clang::fallthrough]]; + #elif defined(_MSC_VER) + __fallthrough; + #endif + case XBOX_DXGI_FORMAT_D16_UNORM_S8_UINT: case XBOX_DXGI_FORMAT_R16_UNORM_X8_TYPELESS: case XBOX_DXGI_FORMAT_X16_TYPELESS_G8_UINT: @@ -1001,6 +1020,11 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height, break; case WIN10_DXGI_FORMAT_V208: + if ((height % 2) != 0) + { + // Requires a height alignment of 2. + return E_INVALIDARG; + } assert(IsPlanar(fmt)); pitch = uint64_t(width); slice = pitch * (uint64_t(height) + (((uint64_t(height) + 1u) >> 1) * 2u));