[master] 3a1fd9bb6 Kill strcat and strcpy usage in VIN_n_Arg
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Tue May 14 09:13:09 UTC 2019
commit 3a1fd9bb611927b47ee45979dcd6781e67bdfc0f
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Tue May 14 00:24:53 2019 +0200
Kill strcat and strcpy usage in VIN_n_Arg
If an absolute path is provided as n_arg with a length of exactly
PATH_MAX-1 then the combination of strcpy and strcat for the trailing
slash '/' overflows dn by one byte, writing its new null-terminating
character '\0' right after dn's upper bound.
By using a fixed-length VSB we can simply ensure that we stay within
bounds at a reasonable cost. Guarding VSB operations should silence
Flexelint as a nice side effect.
VIN_n_Arg is not exposed outside of the source tree, and both callers
today provide a valid dir argument, so we can now make it part of the
contract with an assertion, simplifying the strdup error handling.
diff --git a/lib/libvarnish/vin.c b/lib/libvarnish/vin.c
index 6c098afb7..053e9ed36 100644
--- a/lib/libvarnish/vin.c
+++ b/lib/libvarnish/vin.c
@@ -39,15 +39,19 @@
#include "vdef.h"
-#include "vas.h" // XXX Flexelint "not used" - but req'ed for assert()
+#include "vas.h"
#include "vin.h"
+#include "vsb.h"
int
VIN_n_Arg(const char *n_arg, char **dir)
{
char nm[PATH_MAX];
char dn[PATH_MAX];
+ struct vsb vsb[1];
+ int i;
+ AN(dir);
/* First: determine the name */
@@ -61,25 +65,26 @@ VIN_n_Arg(const char *n_arg, char **dir)
} else
bprintf(nm, "%s", n_arg);
-
/* Second: find the directory name */
+ AN(VSB_new(vsb, dn, sizeof dn, VSB_FIXEDLEN));
+
if (*nm == '/')
- strcpy(dn, nm);
- else if (strlen(VARNISH_STATE_DIR) + 1 + strlen(nm) >= sizeof dn){
- /* preliminary length check to avoid overflowing dm */
+ i = VSB_printf(vsb, "%s/", nm);
+ else
+ i = VSB_printf(vsb, "%s/%s/", VARNISH_STATE_DIR, nm);
+
+ if (i != 0) {
errno = ENAMETOOLONG;
return (-1);
- } else {
- bprintf(dn, "%s/%s", VARNISH_STATE_DIR, nm);
}
- strcat(dn, "/");
+ AZ(VSB_finish(vsb));
+ VSB_clear(vsb);
+
+ *dir = strdup(dn);
+ if (*dir == NULL)
+ return (-1);
- if (dir != NULL) {
- *dir = strdup(dn);
- if (*dir == NULL)
- return (-1);
- }
return (0);
}
More information about the varnish-commit
mailing list