github / github/codeql

False positives in cpp/user-after-free

Đang mở
#19,387 4 bình luận 2 reaction 0 người được giao Xem trên GitHub
acknowledged C++ false-positive
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

`cpp-user-after-free` seems to have a number of false positives, particular when a pointer is `free`d, re-allocated, and then reused correctly.

Consider the following code snippet from [this part](https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/card-piv.c#L2939) of [OpenSC](https://github.com/OpenSC/OpenSC):

```c
free(priv->aid_der.value); /* free previous value if any */
if ((priv->aid_der.value = malloc(resplen)) == NULL) {
LOG_FUNC_RETURN(card->ctx, SC_ERROR_OUT_OF_MEMORY);
}
memcpy(priv->aid_der.value, rbuf, resplen);
priv->aid_der.len = resplen;
LOG_FUNC_RETURN(card->ctx,i);
```

Analysis of the code shows that although free(priv->aid_der.value) is called at line 2939, the pointer priv->aid_der.value is immediately reassigned by a malloc call at line 2940. If the malloc fails, the function returns before the potentially dangerous memcpy at line 2943 is reached. If malloc succeeds, the memcpy operates on the newly allocated buffer, preventing a use-after-free. This finding appears to be a false positive.

Similarly, consider this snippet from [this part](https://github.com/assimp/assimp/blob/master/code/AssetLib/ASE/ASELoader.cpp#L661) of [Assimp](https://github.com/assimp/assimp):

```cpp
delete[] pcScene->mRootNode->mChildren;
for (std::vector::/*const_*/ iterator i = aiList.begin(); i != aiList.end(); ++i) {
const ASE::BaseNode *src = *i;

// The parent is not known, so we can assume that we must add
// this node to the root node of the whole scene
aiNode *pcNode = new aiNode();
pcNode->mParent = pcScene->mRootNode;
pcNode->mName.Set(src->mName);
AddMeshes(src, pcNode);
AddNodes(nodes, pcNode, pcNode->mName.data);
apcNodes.push_back(pcNode);
}

// Regenerate our output array
pcScene->mRootNode->mChildren = new aiNode *[apcNodes.size()];
for (unsigned int i = 0; i < apcNodes.size(); ++i)
pcScene->mRootNode->mChildren[i] = apcNodes[i];

pcScene->mRootNode->mNumChildren = (unsigned int)apcNodes.size();
}
```

Again, the CodeQL finding indicates a potential use-after-free vulnerability where pcScene->mRootNode->mChildren is deleted at line 661 and potentially used later at line 678. Analysis of the code shows that pcScene->mRootNode->mChildren is reallocated with new aiNode*[apcNodes.size()] on line 678 before being accessed in the loop starting on line 679. Therefore, this finding appears to be a false positive, as the memory is valid when accessed.

These findings were generated when running `codeql/cpp-queries` against the given codebases.

## Recommendation

Revise the query to look for use of a `free`d pointer where the the use of the pointer is done before any `malloc`, `new` or similar memory-allocating function is performed. Validate the query against test cases such as these to ensure that proper pointer re-use isn't flagged as a potential UAF.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Start with the cpp/user-after-free query in codeql/cpp-queries and reproduce the C and C++ examples from the issue. Add validation cases for free-then-malloc and delete-then-new pointer reuse, and confirm the query does not report a use after the replacement allocation while still identifying genuine uses before reallocation.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
cpp
Lĩnh vực
devtools, security
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
45/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.