On 30/09/17 00:10, Steffan Karger wrote: > If gc == NULL, the data allocated in the alloc_gc_buf() call in > create_temp_file or the string_mod_const call in gen_path would never > be free'd. > > These functions are currently never called that way, but let's prevent > future problems. > > While touching create_temp_file, also remove the counter variable, which is > never read. > > Signed-off-by: Steffan Karger <[email protected]> > --- > v2: > - change create_temp_file to avoid using a struct buffer (simpler) > - add gc != NULL check for gen_path (avoid similar memleak pitfall) > > src/openvpn/misc.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c > index 90632706..b6f92526 100644 > --- a/src/openvpn/misc.c > +++ b/src/openvpn/misc.c > @@ -723,21 +723,20 @@ test_file(const char *filename) > const char * > create_temp_file(const char *directory, const char *prefix, struct gc_arena > *gc) > { > - static unsigned int counter; > - struct buffer fname = alloc_buf_gc(256, gc); > int fd; > const char *retfname = NULL; > unsigned int attempts = 0; > + char fname[256] = { 0 }; > > do > { > ++attempts; > - ++counter; > > - buf_printf(&fname, PACKAGE "_%s_%08lx%08lx.tmp", prefix, > - (unsigned long) get_random(), (unsigned long) > get_random()); > + openvpn_snprintf(fname, sizeof(fname), PACKAGE "_%s_%08lx%08lx.tmp", > + prefix, (unsigned long) get_random(), > + (unsigned long) get_random());
Minor thought: do we want to check if the return value of snprintf is >=
sizeof(fname)? That would tell us if the string did not entirely fit in
the buffer so that we can abort the whole file creation.
Cheers,
>
> - retfname = gen_path(directory, BSTR(&fname), gc);
> + retfname = gen_path(directory, fname, gc);
> if (!retfname)
> {
> msg(M_WARN, "Failed to create temporary filename and path");
> @@ -812,6 +811,12 @@ gen_path(const char *directory, const char *filename,
> struct gc_arena *gc)
> #else
> const int CC_PATH_RESERVED = CC_SLASH;
> #endif
> +
> + if (!gc)
> + {
> + return NULL; /* Would leak memory otherwise */
> + }
> +
> const char *safe_filename = string_mod_const(filename, CC_PRINT,
> CC_PATH_RESERVED, '_', gc);
>
> if (safe_filename
>
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
