Commit 3a1fd9bb authored by Dridi Boukelmoune's avatar Dridi Boukelmoune

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.
parent 3e02788d
......@@ -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);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment