compressonator icon indicating copy to clipboard operation
compressonator copied to clipboard

Possible buffer overflow in CCodec_ATI2N::Compress

Open aerofly opened this issue 2 years ago • 3 comments

We have observed a buffer overflow in the variable cAlphaBlock[BLOCK_SIZE_4X4] in the function CCodec_ATI2N::Compress in codec_ati2n.cpp at this point:

            if(bUseFixed) {
                CMP_BYTE cAlphaBlock[BLOCK_SIZE_4X4];

                if (bufferIn.m_bSwizzle)
                    bufferIn.ReadBlockB(i*4, j*4, 4, 4, cAlphaBlock);  // <=?? this is actually reading the red channel
                else
                    bufferIn.ReadBlockR(i * 4, j * 4, 4, 4, cAlphaBlock);

When bufferIn is of type CCodecBuffer_RG8, the function ReadBlockB of CCodecBuffer_RG8 is actually writing whn*nPixelSize bytes which equals to 32 bytes and not the 16 bytes ( BLOCK_SIZE_4X4 ) thats actually passed to this function.

Could this be a misuse be on our side or is this indeed a bug? We can easily fix it if we change the variable declaration to

cAlphaBlock[2*BLOCK_SIZE_4X4]

aerofly avatar Jan 24 '23 12:01 aerofly

@aerofly : This looks like a bug in the code

The bufferIn read class CCodecBuffer check for type variances by the data type passed to it.

The input parameter CMP_BYTE cAlphaBlock[BLOCK_SIZE_4X4]; is byte type and the correct read and write CCodecBuffer_... class should then be called based on this.

For this case the ATI2N buffer read will call into this buffer read when input format is 32 bits bool CCodecBuffer_RGBA8888::ReadBlock(CMP_DWORD x, CMP_DWORD y, CMP_BYTE w, CMP_BYTE h, CMP_BYTE block[], CMP_DWORD dwChannelOffset)

When the input buffer is set for 16bits should call bool CCodecBuffer_RG16::ReadBlockR(CMP_DWORD x, CMP_DWORD y, CMP_BYTE w, CMP_BYTE h, CMP_WORD wBlock[]) and not CMP_BYTE

Your fix will correct the buffer overflow.

We will investigate this issue in more details for a permanent fix on all input data types.

Users can find a list of all supported buffer types under the folder \cmp_compressonatorlib\buffer

If your prefer you can use BC5 in cmp_core lib provided input RG channels match that specification (you may have to swap RG channels)

NPCompress avatar Feb 12 '23 17:02 NPCompress

@ NPCompress: Thank you for your detailed answer.

For your reference, the issue arises when we call the function CMP_ConvertTexture( ... ) with a source image with 2 components and 8 bits per component, e.g. source format CMP_FORMAT::CMP_FORMAT_RG_8 and a destination image with destination format CMP_FORMAT::CMP_FORMAT_BC5.

aerofly avatar Feb 14 '23 15:02 aerofly

@aerofly Thanks for the added info, will work on a update for the issue.

NPCompress avatar Feb 14 '23 15:02 NPCompress