[PATCH] D97699: [analyzer] Add InvalidPtrChecker

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze added inline comments.


================
Comment at: clang/docs/analyzer/checkers.rst:2103-2104
+        puts(envp[i]);
+        // envp may no longer point to the current environment
+        // this program has unanticipated behavior.
+      }
----------------
zukatsinadze wrote:
> Charusso wrote:
> > NoQ wrote:
> > > It's very important to explain whether the "unanticipated behavior" is an entirely psychological concept ("most people don't understand how this works but this can occasionally also be a valid solution if you know what you're doing") or a 100% clear indication of a bug ("such code can never be correct", eg. it instantly causes undefined behavior, or the written value can never be read later so there's absolutely no point in writing it, or something like that).
> > The standard terminology is very vague, like that:
> > ```
> > The getenv function returns a pointer to a string associated with the matched list member. The string pointed to shall not be modified by the program but may be overwritten by a subsequent call to the getenv function.
> > ```
> > What the hell. 😕  I think it is about table-doubling and reallocating the entire environment pointer table at some point which makes sense in case of the non-getter function calls. For the getters I think another processes could overwrite the `environ` pointer between two getter calls and problem could occur because of such table-doubling. To resolve the issue we need to call `strdup()` and create a copy of the current environment entry instead of having a pointer to it as I see.
> > 
> > @zukatsinadze it would be great to see a real reference with real issues in real world software. Is the true evil the multiple chained getter calls?
> > 
> > it would be great to see a real reference with real issues in real world software
> 
> I've attached some results up in the thread. Checker gave several valuable reports on several projects if that's what you mean.
> 
> >  Is the true evil the multiple chained getter calls?
> 
> Besides SEI CERT, there are many other reputable resources stating the same problem with getenv.
> 
> https://www.ibm.com/support/knowledgecenter/SSLTBW_2.2.0/com.ibm.zos.v2r2.bpxbd00/getenv.htm
> https://www.keil.com/support/man/docs/armlib/armlib_chr1359122850667.htm
> https://pubs.opengroup.org/onlinepubs/009696799/functions/getenv.html
> https://pubs.opengroup.org/onlinepubs/7908799/xsh/getenv.html
> https://www.gnu.org/software/libc/manual/html_node/Environment-Access.html
> https://man7.org/linux/man-pages/man3/getenv.3p.html
Actual problem is that getenv is returning a pointer to its internal static buffer instead of giving pointer to environ, that's why the string will change with a subsequent call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97699