https://github.com/DonatNagyE commented:
These are small and straightforward improvements, they mostly LGTM. Note that in the commit message of "[[analyzer] Fix stdin declaration in C++ tests](https://github.com/llvm/llvm-project/pull/66074/commits/ededc22487a23a7deaa971526da8a932ea27b231)" the first line contains a typo ("shoulkd"). On the commit "[[analyzer] Fix taint sink rules for exec-like functions](https://github.com/llvm/llvm-project/pull/66074/commits/c54e1342fe93a51acce9148072d60d8783333194)" I think that the old behavior might be intentional: it reports situations when malicious actors may influence which executable will be ran, but does not report situations when the arguments passed to a (presumably trusted) executable contain user input. Of course, there are situations where tainted arguments are also very problematic (e.g. calling `sudo`, `sh` or similar "execute something" programs), and I can accept this as a policy change, but I wouldn't call it a "Fix" of outright buggy behavior. On the meta level, I'd also remark that this "four commits in one batch" review is cumbersome (at least for me -- I'm not accustomed to github gui), and I'd prefer to see separate pull requests for unrelated commits if that's not a serious problem for you. In summary, I'd say that: - [[analyzer] Make socket accept() propagate taint](https://github.com/llvm/llvm-project/pull/66074/commits/b3215f7b2e5df91bb0a12cbe309929ee9d764b16) and [[analyzer] Propagate taint for wchar variants of some APIs](https://github.com/llvm/llvm-project/pull/66074/commits/63b6a37624935ae0bb52b830e15b015998559e06) definitely LGTM; feel free to push them separately. - On [[analyzer] Fix stdin declaration in C++ tests](https://github.com/llvm/llvm-project/pull/66074/commits/ededc22487a23a7deaa971526da8a932ea27b231) I have an inline question but the change is acceptable as it's now. - On [[analyzer] Fix taint sink rules for exec-like functions](https://github.com/llvm/llvm-project/pull/66074/commits/c54e1342fe93a51acce9148072d60d8783333194) I'd wait a bit to see the opinion of others about handling the `exec...` functions in a stricter manner. https://github.com/llvm/llvm-project/pull/66074 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
