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 <stef...@karger.me>
> ---
> 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 Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
