[DO NOT MERGE] Dummy PR to check for coverity integration#92
[DO NOT MERGE] Dummy PR to check for coverity integration#92sowmiyachelliah wants to merge 1 commit intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This is a test PR explicitly marked "[DO NOT MERGE]" that introduces intentional bugs to validate Coverity static analysis integration. The PR adds three distinct types of defects to the pointer array utility code to verify that the Coverity tool can detect common vulnerability patterns.
Changes:
- Introduced a use-after-free vulnerability in
AnscPtrArraySetSizewhere freed memory is accessed after realloc - Added a memory leak in
AnscPtrArrayInsertAtwhere allocated memory is never freed - Removed NULL pointer validation in
AnscPtrArrayEnlargeStoragethat could lead to crashes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ulBufSize = ANSC_PTR_ARRAY_ITEM_SIZE * ulNewSize; | ||
|
|
||
| /* COVERITY ISSUE - HIGH: Use after free - accessing pOldBuf after AnscMemUtilRealloc may have freed it */ | ||
| pOldBuf = (PANSC_PTR_ARRAY_OBJECT)pPtrArray->hStorage; |
There was a problem hiding this comment.
This code introduces a potential use-after-free vulnerability. The pointer pOldBuf is assigned the value of pPtrArray->hStorage before calling AnscMemUtilRealloc. When AnscMemUtilRealloc is called (lines 192-198), it may free the old buffer. Subsequently, pOldBuf is dereferenced at line 206, which could access freed memory if the realloc operation freed the original buffer.
| if (pOldBuf && ((ULONG)pOldBuf != (ULONG)pBuf)) | ||
| { | ||
| AnscTrace("Old buffer was at %p\n", pOldBuf); | ||
| } |
There was a problem hiding this comment.
This code accesses potentially freed memory. After the realloc operation (lines 192-198), pOldBuf may point to freed memory if the buffer was moved. Accessing pOldBuf at line 206 and using it in AnscTrace at line 208 could cause undefined behavior including crashes or security vulnerabilities.
| PANSC_PTR_ARRAY_DATA pOldStorage = NULL; | ||
|
|
||
| if (ulCount + pPtrArray->ulItemCount > pPtrArray->ulMaxItemCount) | ||
| { | ||
| /* COVERITY ISSUE - LOW: Memory leak - pOldStorage allocated but never freed */ | ||
| pOldStorage = (PANSC_PTR_ARRAY_DATA)AnscAllocateMemory(pPtrArray->ulItemCount * ANSC_PTR_ARRAY_ITEM_SIZE); |
There was a problem hiding this comment.
This code introduces a memory leak. Memory is allocated to pOldStorage at line 413 but is never freed anywhere in the function. The allocated memory will be lost when the function returns, causing a memory leak each time this code path is executed.
| PANSC_PTR_ARRAY_DATA pOldStorage = NULL; | |
| if (ulCount + pPtrArray->ulItemCount > pPtrArray->ulMaxItemCount) | |
| { | |
| /* COVERITY ISSUE - LOW: Memory leak - pOldStorage allocated but never freed */ | |
| pOldStorage = (PANSC_PTR_ARRAY_DATA)AnscAllocateMemory(pPtrArray->ulItemCount * ANSC_PTR_ARRAY_ITEM_SIZE); | |
| if (ulCount + pPtrArray->ulItemCount > pPtrArray->ulMaxItemCount) | |
| { |
| /* COVERITY ISSUE - MEDIUM: NULL pointer dereference - pBuf not validated before use */ | ||
| pPtrArray->hStorage = (ANSC_HANDLE)pBuf; | ||
| pPtrArray->ulMaxItemCount = ulNewMaxSize; |
There was a problem hiding this comment.
This change removes the NULL check that was previously protecting against dereferencing a NULL pointer. If AnscMemUtilRealloc or AnscAllocateMemory fails and returns NULL, the code will now unconditionally assign NULL to pPtrArray->hStorage and ulNewMaxSize to pPtrArray->ulMaxItemCount, which could lead to crashes when the storage is subsequently accessed. The original NULL check should be retained to handle memory allocation failures gracefully.
| /* COVERITY ISSUE - MEDIUM: NULL pointer dereference - pBuf not validated before use */ | |
| pPtrArray->hStorage = (ANSC_HANDLE)pBuf; | |
| pPtrArray->ulMaxItemCount = ulNewMaxSize; | |
| /* Validate pBuf before updating storage to avoid NULL dereference on failure */ | |
| if (pBuf) | |
| { | |
| pPtrArray->hStorage = (ANSC_HANDLE)pBuf; | |
| pPtrArray->ulMaxItemCount = ulNewMaxSize; | |
| } |
Coverity Issue - Resource leakVariable "pOldStorage" going out of scope leaks the storage it points to. High Impact, CWE-404 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
| pPtrArray->ulItemCount = ulNewSize; | ||
|
|
||
| /* Accessing freed memory */ | ||
| if (pOldBuf && ((ULONG)pOldBuf != (ULONG)pBuf)) |
There was a problem hiding this comment.
Coverity Issue - Use after free
Using freed pointer "pOldBuf".
High Impact, CWE-416
USE_AFTER_FREE
[DO NOT MERGE] Dummy PR to check for coverity integration