[gnutls-devel] GnuTLS | tools: suppress ctime() error from lgtm warnings (!994)

[Development of GNU's TLS library]

Tim Rühsen started a new discussion on src/certtool.c: https://gitlab.com/gnutls/gnutls/merge_requests/994#note_168214561

>  
>  		if (ca_crt && (secs > gnutls_x509_crt_get_expiration_time(ca_crt))) {
>  			time_t exp = gnutls_x509_crt_get_expiration_time(ca_crt);
> -			fprintf(stderr, "\nExpiration time: %s", ctime(&secs));
> -			fprintf(stderr, "CA expiration time: %s", ctime(&exp));
> +			fprintf(stderr, "\nExpiration time: %s", ctime(&secs)); //lgtm [cpp/potentially-dangerous-function]

I am against adding suppressions for a single static analyzer service like LGTM in the source code. Consequently you also have to add suppressions for all the other static analyzers out there, which blows up code and make it unreadable. As an exception I would agree to suppressions for gcc+clang as they are basic tools that we likely use forever. Services like LGTM come and go.

In this case I would even throw in that ctime() indeed should be avoided. Just think of copy&pasting code into a multi-threaded application or library. After an RCE, someone will ask "where did this code come from ? Oh from GnuTLS - what a crap !".

Working around ctime() will also silence *any* static analyzer. In this case ctime_r() seems appropriate. But it doesn't provide a buffer length (the docs say the buffer has to be 26 bytes at least - but what if you are above year 9999 ?).

So in the end (if locale support is needed), a small helper using strftime() sems to be the best solution.

-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/merge_requests/994#note_168214561
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20190509/27208578/attachment.html>