codeql icon indicating copy to clipboard operation
codeql copied to clipboard

False positives in cpp/use-after-free

Open parsley72 opened this issue 1 year ago • 1 comments

I'm getting about 20 of these on my code but I'll only show one because they're all the same.

CodeQL is identifying this as a potential use after free because there's a delete[] on line 3 of this sample. But line 5 calls new[] so the pointer used on line 6 should never be dangerous to use. I don't know if CodeQL is confused by the number of pointers but this isn't an issue.

Step 1 pointer to operator delete[] output argument Step 2 *m_CSEngine [post update] [m_arrSpeedInit] Step 3 *m_pRaw [post update] [*m_CSEngine, m_arrSpeedInit] Step 4 *this [post update] [*m_pRaw, *m_CSEngine, m_arrSpeedInit] Step 5 *this [*m_pRaw, *m_CSEngine, m_arrSpeedInit] Step 6 *m_pRaw [*m_CSEngine, m_arrSpeedInit] Step 7 *m_CSEngine [m_arrSpeedInit] Step 8 m_arrSpeedInit - Memory may have been previously freed by delete[].

		if (((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_arrSpeedInit!=NULL)
		{
			delete []((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_arrSpeedInit; // <- Steps 1, 2, 3, 4
		}
		((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_arrSpeedInit = new int[((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_numberObj];
		memcpy(((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_arrSpeedInit,(BYTE*)p+pos,((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_numberObj*sizeof(int)); // <- Steps 5, 6, 7, 8

parsley72 avatar Feb 21 '24 02:02 parsley72

Hi @parsley72,

Thanks for detailed reporting on this. I've opened an internal issue to track this. It looks like our dataflow library fails to conclude that ((CRawDetectionPlate*)m_pRaw)->m_CSEngine->m_arrSpeedInit = new int[...] clears the current value of m_arrSpeedInit, and thus the "deleted value" coming out of delete [] ... incorrectly reaches the memcpy argument.

I'll discuss with the team how to handle this, and I'll let you know as soon as we have a fix 🙂

MathiasVP avatar Feb 22 '24 13:02 MathiasVP