github / github/codeql

False positive - cpp/constant-comparison

未關閉
#12,399 2 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
acknowledged C++ false-positive
主要語言
CodeQL
星號
10.1k
分支
2.1k
平均合併
2 天 15 小時
30 天內合併 PR
141

描述

**Description of the false positive**

https://github.com/ryao/zfs/security/code-scanning/275

https://github.com/ryao/zfs/blob/7645b3fc537a7c2960100b3e8da2dec13688b563/lib/libzutil/zutil_import.c#L662-L665

`cpp/constant-comparison` reports `Comparison is always false because c >= 0 and 0 >= children.`, but the second condition is not true. We can set children to a larger value on the line `children = id + 1`. Let us assume that `id` is 0 in this case, such that `children` becomes 1. Immediately afterward, we can run `goto nomem;` following an error from `nvlist_dup()`. Then execution will reach `for (c = 0; c < children; c++)`, where the check suggests that `c < children` is always false, when in reality, it will be true.

Ignoring the false positive, there is another issue with this report. It tells us that `0 >= children` is always true. That could lead one to think that we could have negative values, but `children` is unsigned, so it would have been better to recognize that this is an unsigned type and say `0 == children` is always true. I had to check the type because of that lack of precision, since while `0 >= children` is a valid statement even with unsigned integers, it is not intuitive for a human reading it. Of course, in this situation, the claim that `0 == children` is always true would still be wrong, but it would have made the report slightly less confusing.

That said, I understand that this function is fairly ugly and could stand some refactoring, but CodeQL's query should not be confused by that.

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。