[PATCH] D120236: [analyzer] Add more sources to Taint analysis

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Fewer nits this time.
We are converging!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:559
+      {{"gethostname"}, TR::Source({{0}})},
+      {{"getnameinfo"}, TR::Source({{2, 4}})},
+      {{"getseuserbyname"}, TR::Source({{1, 2}})},
----------------
gamesh411 wrote:
> steakhal wrote:
> > In what cases can this function introduce taint?
> The getnameinfo converts from
> ```
> struct sockaddr_in {
>     sa_family_t    sin_family; /* address family: AF_INET */
>     in_port_t      sin_port;   /* port in network byte order */
>     struct in_addr sin_addr;   /* internet address */
> };
> 
> /* Internet address */
> struct in_addr {
>     uint32_t       s_addr;     /* address in network byte order */
> };
> ```
> to hostname and servername strings.
> One could argue that by crafting a specific IP address, that  is known to resolve to a specific hostname in the running environment could lead an attacker injecting a chosen (in some circumstances arbitrary) string into the code at the point of this function.
> 
> I know this is a bit contrived, and more on the cybersecurity side of things, so I am not sure whether to add this here, or add this in a specific checker, or just leave altogether. Please share your opinion about this.
Let it be, I don't mind. We will remove it if we find some FPs for this.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:560
+      {{"readlink"}, TR::Source({{1, ReturnValueIndex}})},
+      {{"readlinkat"}, TR::Source({{1, ReturnValueIndex}})},
+      {{"get_current_dir_name"}, TR::Source({{ReturnValueIndex}})},
----------------
`int readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsiz);`


================
Comment at: clang/test/Analysis/taint-generic.c:372
+int _IO_getc(_IO_FILE *__fp);
+int test_IO_getc(_IO_FILE *fp) {
+  char c = _IO_getc(fp);
----------------
Please, also rename this test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120236/new/

https://reviews.llvm.org/D120236