[PATCH v2] sunrpc: use snprintf instead of an implied length-guarantee on a format argument
Philipp Tomsich
philipp.tomsich@vrull.eu
Thu Dec 3 12:22:32 GMT 2020
GCC11 has improved detection of buffer overflows detectable through the analysis
of format strings and parameters, which identifies the following issue:
netname.c:52:28: error: '%s' directive writing up to 255 bytes into a region
of size between 239 and 249 [-Werror=format-overflow=]
While an if-check prior to the format-directive implies a guarantee on the length
of the respective format argument, the compiler does not derive a range from that
implied guarantuee and does not consider the range in performing the checking.
This rewrites user2netname() to use snprintf to guard against overflows.
In doing so, it removes the if-check and related macros.
---
Changes in v2:
- Use ssize_t to hold the return from snprintf and check for negative return
values indicating an error.
- Remove the if-statement that checks the length of the format arguments and
provides an implied guarantuee.
- Reword the commit message to clearly state that this does not address an
exploitable overflow (as there is an implied guarantuee on the length of the
string from a previous if-statement).
sunrpc/netname.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sunrpc/netname.c b/sunrpc/netname.c
index 24ee519..251ab94 100644
--- a/sunrpc/netname.c
+++ b/sunrpc/netname.c
@@ -24,8 +24,7 @@
#include "nsswitch.h"
-#define OPSYS_LEN 4
-#define MAXIPRINT (11) /* max length of printed integer */
+#define OPSYS_LEN 4
static const char OPSYS[] = "unix";
int
@@ -33,7 +32,7 @@ user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
const char *domain)
{
char dfltdom[MAXNETNAMELEN + 1];
- size_t i;
+ ssize_t i;
if (domain == NULL)
{
@@ -46,11 +45,10 @@ user2netname (char netname[MAXNETNAMELEN + 1], const uid_t uid,
dfltdom[MAXNETNAMELEN] = '\0';
}
- if ((strlen (dfltdom) + OPSYS_LEN + 3 + MAXIPRINT) > (size_t) MAXNETNAMELEN)
+ i = snprintf (netname, MAXNETNAMELEN + 1, "%s.%d@%s", OPSYS, uid, dfltdom);
+ if (i <= 0 || i > (ssize_t) MAXNETNAMELEN)
return 0;
- sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
- i = strlen (netname);
if (netname[i - 1] == '.')
netname[i - 1] = '\0';
return 1;
--
1.8.3.1
More information about the Libc-alpha
mailing list