[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: sharutils does not build with -Werror=format-security
From: |
Bruce Korb |
Subject: |
Re: sharutils does not build with -Werror=format-security |
Date: |
Sat, 12 Oct 2013 15:26:05 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130922 Icedove/17.0.9 |
On 10/12/13 08:21, Santiago Vila wrote:
Hello.
The following patch is required to allow compilation of
sharutils 4.13.5 with -Werror=format-security.
Thanks.
Hi Santiago, Simon, et al.
OK. I give: what possible good is -Werror=format-security?
According to the doc, it is dangerous to use variables as formats
because the variable "just might" contain a %n and wreak havoc.
That makes i18n an interesting challenge. If the format contains
translatable text, the formatting string *MUST* not be static.
In other words, code like this:
static int
check_accessibility (const char *local_name, const char *restore_name)
{
if (access (local_name, 4))
{
error (0, errno, _("Cannot access %s"), local_name);
return SHAR_EXIT_FILE_NOT_FOUND;
}
must be left alone. If that has to be left alone, then:
@@ -954,7 +954,7 @@
free (c_dir);
}
else
- error (0, errno, _("Cannot get current directory name"));
+ error (0, errno, "%s", _("Cannot get current directory name"));
}
}
this change is pointless nonsense. I think GCC went off the rails when
they warned "OMG! You have a NUL byte in the format string" and this
is again an overkill. If someone switches out the l10n library for
a bogus one, then you get the old garbage-in-garbage-out problem.
Where the third argument to error is, in fact, some variable string,
I will pass it through a "%s" format specifier. I suspect they should
all be validated as clean, but that patch is easier than verifying
validity. Compile with this warning set IFF NLS is disabled.
If Debian/GCC still complains, then the warning should be silenced.
diff --git a/src/shar.c b/src/shar.c
index f12cf19..3ce860c 100644
--- a/src/shar.c
+++ b/src/shar.c
@@ -444,7 +444,7 @@ walkdown (
if (stat (local_name, &struct_stat))
{
- error (0, errno, local_name);
+ error (0, errno, "%s", local_name);
return SHAR_EXIT_FILE_NOT_FOUND;
}
@@ -453,7 +453,7 @@ walkdown (
if (directory = opendir (local_name), !directory)
{
- error (0, errno, local_name);
+ error (0, errno, "%s", local_name);
return SHAR_EXIT_CANNOT_OPENDIR;
}
@@ -535,7 +535,7 @@ walkdown (
#else
if (closedir (directory))
{
- error (0, errno, local_name);
+ error (0, errno, "%s", local_name);
return SHAR_EXIT_CANNOT_OPENDIR;
}
#endif
@@ -587,7 +587,7 @@ walktree (walker_t routine, const char *local_name)
if (status != 0)
{
- error (0, errno, local_name_copy);
+ error (0, errno, "%s", local_name_copy);
status = SHAR_EXIT_FILE_NOT_FOUND;
}
else
@@ -2368,7 +2368,7 @@ main (int argc, char ** argv)
optionLoadLine (&sharOptions, arg);
}
else
- error (0, errno, arg);
+ error (0, errno, "%s", arg);
continue;
}