[Openvpn-devel] Silence warning about format string in check_ca_required

Message ID 20210608194330.1431202-1-arne@rfc2549.org
State Superseded
Headers show
Series
  • [Openvpn-devel] Silence warning about format string in check_ca_required
Related show

Commit Message

Arne Schwabe June 8, 2021, 7:43 p.m.
clang does not like if the format argument of printf like function
is not a string literal:

warning: format string is not a string literal (potentially insecure)

Use "%s" as string literal to silence the warning.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Antonio Quartulli June 8, 2021, 7:59 p.m. | #1
Hi,

On 08/06/2021 21:43, Arne Schwabe wrote:
> clang does not like if the format argument of printf like function
> is not a string literal:
> 
> warning: format string is not a string literal (potentially insecure)
> 
> Use "%s" as string literal to silence the warning.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/options.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 8978955c3..5ecb7b7db 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -2077,7 +2077,7 @@ check_ca_required(const struct options *options)
>          " or CA path (--capath)"
>  #endif
>          " and/or peer fingerprint verification (--peer-fingerprint)";
> -    msg(M_USAGE, str);
> +    msg(M_USAGE, "%s", str);

Imho this warning is a borderline one.

Rather than using %s as clang suggests, I am more in favor of the following:


diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 819979b1..086f7b6e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2069,7 +2069,7 @@ check_ca_required(const struct options *options)
         return;
     }

-    const char* str = "You must define CA file (--ca)"
+    const char* const str = "You must define CA file (--ca)"
 #ifndef ENABLE_CRYPTO_MBEDTLS
         " or CA path (--capath)"
 #endif



Passing "%s" just to make clang happy looks weird to me.

Regards,
Selva Nair June 8, 2021, 8:09 p.m. | #2
Hi

On Tue, Jun 8, 2021 at 3:44 PM Arne Schwabe <arne@rfc2549.org> wrote:

> clang does not like if the format argument of printf like function
> is not a string literal:
>
> warning: format string is not a string literal (potentially insecure)
>
> Use "%s" as string literal to silence the warning.
>

Format string is often not a string literal -- we have many other such
usages in the source, though probably none without a third argument. grep
for fmt, for example.  Here str is a const char *, what more do they want?
Would const char * const str work?

Selva
<div dir="ltr"><div>Hi</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 8, 2021 at 3:44 PM Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">clang does not like if the format argument of printf like function<br>
is not a string literal:<br>
<br>
warning: format string is not a string literal (potentially insecure)<br>
<br>
Use &quot;%s&quot; as string literal to silence the warning.<br></blockquote><div><br></div><div>Format string is often not a string literal -- we have many other such usages in the source, though probably none without a third argument. grep for fmt, for example.  Here str is a const char *, what more do they want? Would const char * const str work?</div><div><br></div><div>Selva</div></div></div>

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8978955c3..5ecb7b7db 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2077,7 +2077,7 @@  check_ca_required(const struct options *options)
         " or CA path (--capath)"
 #endif
         " and/or peer fingerprint verification (--peer-fingerprint)";
-    msg(M_USAGE, str);
+    msg(M_USAGE, "%s", str);
 }
 
 static void