Skip to content

Commit

Permalink
Remove openvpn_snprintf and similar functions
Browse files Browse the repository at this point in the history
Old Microsoft versions did strange behaviour but according to the
newly added unit test and
https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating
this is now standard conforming and we can use the normal snprintf
method.

Microsoft own documentation to swprintf also says you nowadays need to
define _CRT_NON_CONFORMING_SWPRINTFS to get to non-standard behaviour.

Change-Id: I07096977e3b562bcb5d2c6f11673a4175b8e12ac
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240506102710.8976-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28617.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
  • Loading branch information
schwabe authored and cron2 committed May 6, 2024
1 parent 9d92221 commit 130548f
Show file tree
Hide file tree
Showing 38 changed files with 291 additions and 333 deletions.
26 changes: 0 additions & 26 deletions src/openvpn/buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,32 +279,6 @@ buf_puts(struct buffer *buf, const char *str)
return ret;
}


/*
* This is necessary due to certain buggy implementations of snprintf,
* that don't guarantee null termination for size > 0.
*
* Return false on overflow.
*
* This functionality is duplicated in src/openvpnserv/common.c
* Any modifications here should be done to the other place as well.
*/

bool
openvpn_snprintf(char *str, size_t size, const char *format, ...)
{
va_list arglist;
int len = -1;
if (size > 0)
{
va_start(arglist, format);
len = vsnprintf(str, size, format, arglist);
va_end(arglist);
str[size - 1] = 0;
}
return (len >= 0 && len < size);
}

/*
* write a string to the end of a buffer that was
* truncated by buf_printf
Expand Down
13 changes: 0 additions & 13 deletions src/openvpn/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,19 +448,6 @@ __attribute__ ((format(__printf__, 2, 3)))
*/
bool buf_puts(struct buffer *buf, const char *str);

/*
* Like snprintf but guarantees null termination for size > 0
*/
bool openvpn_snprintf(char *str, size_t size, const char *format, ...)
#ifdef __GNUC__
#if __USE_MINGW_ANSI_STDIO
__attribute__ ((format(gnu_printf, 3, 4)))
#else
__attribute__ ((format(__printf__, 3, 4)))
#endif
#endif
;


/*
* remove/add trailing characters
Expand Down
4 changes: 2 additions & 2 deletions src/openvpn/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -874,11 +874,11 @@ init_key_ctx_bi(struct key_ctx_bi *ctx, const struct key2 *key2,

key_direction_state_init(&kds, key_direction);

openvpn_snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name);
snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name);
init_key_ctx(&ctx->encrypt, &key2->keys[kds.out_key], kt,
OPENVPN_OP_ENCRYPT, log_prefix);

openvpn_snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name);
snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name);
init_key_ctx(&ctx->decrypt, &key2->keys[kds.in_key], kt,
OPENVPN_OP_DECRYPT, log_prefix);

Expand Down
10 changes: 5 additions & 5 deletions src/openvpn/crypto_mbedtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ mbed_log_func_line(unsigned int flags, int errval, const char *func,
{
char prefix[256];

if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line))
if (!snprintf(prefix, sizeof(prefix), "%s:%d", func, line))
{
return mbed_log_err(flags, errval, func);
}
Expand Down Expand Up @@ -239,11 +239,11 @@ crypto_pem_encode(const char *name, struct buffer *dst,
char header[1000+1] = { 0 };
char footer[1000+1] = { 0 };

if (!openvpn_snprintf(header, sizeof(header), "-----BEGIN %s-----\n", name))
if (!snprintf(header, sizeof(header), "-----BEGIN %s-----\n", name))
{
return false;
}
if (!openvpn_snprintf(footer, sizeof(footer), "-----END %s-----\n", name))
if (!snprintf(footer, sizeof(footer), "-----END %s-----\n", name))
{
return false;
}
Expand Down Expand Up @@ -278,11 +278,11 @@ crypto_pem_decode(const char *name, struct buffer *dst,
char header[1000+1] = { 0 };
char footer[1000+1] = { 0 };

if (!openvpn_snprintf(header, sizeof(header), "-----BEGIN %s-----", name))
if (!snprintf(header, sizeof(header), "-----BEGIN %s-----", name))
{
return false;
}
if (!openvpn_snprintf(footer, sizeof(footer), "-----END %s-----", name))
if (!snprintf(footer, sizeof(footer), "-----END %s-----", name))
{
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/openvpn/dns.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,11 +349,11 @@ setenv_dns_option(struct env_set *es,

if (j < 0)
{
name_ok = openvpn_snprintf(name, sizeof(name), format, i);
name_ok = snprintf(name, sizeof(name), format, i);
}
else
{
name_ok = openvpn_snprintf(name, sizeof(name), format, i, j);
name_ok = snprintf(name, sizeof(name), format, i, j);
}

if (!name_ok)
Expand Down
8 changes: 4 additions & 4 deletions src/openvpn/env_set.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,23 @@ void
setenv_counter(struct env_set *es, const char *name, counter_type value)
{
char buf[64];
openvpn_snprintf(buf, sizeof(buf), counter_format, value);
snprintf(buf, sizeof(buf), counter_format, value);
setenv_str(es, name, buf);
}

void
setenv_int(struct env_set *es, const char *name, int value)
{
char buf[64];
openvpn_snprintf(buf, sizeof(buf), "%d", value);
snprintf(buf, sizeof(buf), "%d", value);
setenv_str(es, name, buf);
}

void
setenv_long_long(struct env_set *es, const char *name, long long value)
{
char buf[64];
openvpn_snprintf(buf, sizeof(buf), "%" PRIi64, (int64_t)value);
snprintf(buf, sizeof(buf), "%" PRIi64, (int64_t)value);
setenv_str(es, name, buf);
}

Expand Down Expand Up @@ -310,7 +310,7 @@ setenv_str_incr(struct env_set *es, const char *name, const char *value)
strcpy(tmpname, name);
while (NULL != env_set_get(es, tmpname) && counter < 1000)
{
ASSERT(openvpn_snprintf(tmpname, tmpname_len, "%s_%u", name, counter));
ASSERT(snprintf(tmpname, tmpname_len, "%s_%u", name, counter));
counter++;
}
if (counter < 1000)
Expand Down
14 changes: 7 additions & 7 deletions src/openvpn/error.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,14 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)

if ((flags & M_ERRNO) && e)
{
openvpn_snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)",
m1, openvpn_strerror(e, crt_error, &gc), e);
snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)",
m1, openvpn_strerror(e, crt_error, &gc), e);
SWAP;
}

if (flags & M_OPTERR)
{
openvpn_snprintf(m2, ERR_BUF_SIZE, "Options error: %s", m1);
snprintf(m2, ERR_BUF_SIZE, "Options error: %s", m1);
SWAP;
}

Expand Down Expand Up @@ -321,10 +321,10 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)
const struct virtual_output *vo = msg_get_virtual_output();
if (vo)
{
openvpn_snprintf(m2, ERR_BUF_SIZE, "%s%s%s",
prefix,
prefix_sep,
m1);
snprintf(m2, ERR_BUF_SIZE, "%s%s%s",
prefix,
prefix_sep,
m1);
virtual_output_print(vo, flags, m2);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/openvpn/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ management_callback_remote_entry_get(void *arg, unsigned int index, char **remot
char *out = malloc(len);
check_malloc_return(out);

openvpn_snprintf(out, len, "%s,%s,%s,%s", ce->remote, ce->remote_port, proto, status);
snprintf(out, len, "%s,%s,%s,%s", ce->remote, ce->remote_port, proto, status);
*remote = out;
}
else
Expand Down
8 changes: 4 additions & 4 deletions src/openvpn/manage.c
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,8 @@ man_bytecount_output_client(struct management *man,
char out[32];

/* do in a roundabout way to work around possible mingw or mingw-glibc bug */
openvpn_snprintf(in, sizeof(in), counter_format, man->persist.bytes_in + dco_read_bytes);
openvpn_snprintf(out, sizeof(out), counter_format, man->persist.bytes_out + dco_write_bytes);
snprintf(in, sizeof(in), counter_format, man->persist.bytes_in + dco_read_bytes);
snprintf(out, sizeof(out), counter_format, man->persist.bytes_out + dco_write_bytes);
msg(M_CLIENT, ">BYTECOUNT:%s,%s", in, out);
}

Expand All @@ -528,8 +528,8 @@ man_bytecount_output_server(const counter_type *bytes_in_total,
char in[32];
char out[32];
/* do in a roundabout way to work around possible mingw or mingw-glibc bug */
openvpn_snprintf(in, sizeof(in), counter_format, *bytes_in_total);
openvpn_snprintf(out, sizeof(out), counter_format, *bytes_out_total);
snprintf(in, sizeof(in), counter_format, *bytes_in_total);
snprintf(out, sizeof(out), counter_format, *bytes_out_total);
msg(M_CLIENT, ">BYTECOUNT_CLI:%lu,%s,%s", mdac->cid, in, out);
mdac->bytecount_last_update = now;
}
Expand Down
8 changes: 4 additions & 4 deletions src/openvpn/options.c
Original file line number Diff line number Diff line change
Expand Up @@ -1427,7 +1427,7 @@ foreign_options_copy_dns(struct options *o, struct env_set *es)
for (int i = 1; i <= opt_max; ++i)
{
char name[32];
openvpn_snprintf(name, sizeof(name), "foreign_option_%d", i);
snprintf(name, sizeof(name), "foreign_option_%d", i);

const char *env_str = env_set_get(es, name);
const char *value = strchr(env_str, '=') + 1;
Expand Down Expand Up @@ -1482,7 +1482,7 @@ foreign_options_copy_dns(struct options *o, struct env_set *es)
while (o->foreign_option_index < opt_max)
{
char name[32];
openvpn_snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
setenv_del(es, name);
}
}
Expand Down Expand Up @@ -5674,8 +5674,8 @@ set_user_script(struct options *options,
#ifndef ENABLE_SMALL
{
char script_name[100];
openvpn_snprintf(script_name, sizeof(script_name),
"--%s script", type);
snprintf(script_name, sizeof(script_name),
"--%s script", type);

if (check_cmd_access(*script, script_name, (in_chroot ? options->chroot_dir : NULL)))
{
Expand Down
6 changes: 3 additions & 3 deletions src/openvpn/pkcs11.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ _pkcs11_openvpn_token_prompt(
CLEAR(token_resp);
token_resp.defined = false;
token_resp.nocache = true;
openvpn_snprintf(
snprintf(
token_resp.username,
sizeof(token_resp.username),
"Please insert %s token",
Expand Down Expand Up @@ -245,7 +245,7 @@ _pkcs11_openvpn_pin_prompt(

ASSERT(token!=NULL);

openvpn_snprintf(prompt, sizeof(prompt), "%s token", token->label);
snprintf(prompt, sizeof(prompt), "%s token", token->label);

token_pass.defined = false;
token_pass.nocache = true;
Expand Down Expand Up @@ -719,7 +719,7 @@ tls_ctx_use_pkcs11(

id_resp.defined = false;
id_resp.nocache = true;
openvpn_snprintf(
snprintf(
id_resp.username,
sizeof(id_resp.username),
"Please specify PKCS#11 id to use"
Expand Down
6 changes: 3 additions & 3 deletions src/openvpn/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,9 @@ platform_create_temp_file(const char *directory, const char *prefix, struct gc_a
{
++attempts;

if (!openvpn_snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len,
prefix, (unsigned long) get_random(),
(unsigned long) get_random()))
if (!snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len,
prefix, (unsigned long) get_random(),
(unsigned long) get_random()))
{
msg(M_WARN, "ERROR: temporary filename too long");
return NULL;
Expand Down
4 changes: 2 additions & 2 deletions src/openvpn/plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ plugin_init_item(struct plugin *p, const struct plugin_option *o)
{
char full[PATH_MAX];

openvpn_snprintf(full, sizeof(full), "%s/%s", PLUGIN_LIBDIR, p->so_pathname);
snprintf(full, sizeof(full), "%s/%s", PLUGIN_LIBDIR, p->so_pathname);
p->handle = dlopen(full, RTLD_NOW);
}
else
Expand Down Expand Up @@ -409,7 +409,7 @@ plugin_vlog(openvpn_plugin_log_flags_t flags, const char *name, const char *form

gc_init(&gc);
msg_fmt = gc_malloc(ERR_BUF_SIZE, false, &gc);
openvpn_snprintf(msg_fmt, ERR_BUF_SIZE, "PLUGIN %s: %s", name, format);
snprintf(msg_fmt, ERR_BUF_SIZE, "PLUGIN %s: %s", name, format);
x_msg_va(msg_flags, msg_fmt, arglist);

gc_free(&gc);
Expand Down
2 changes: 1 addition & 1 deletion src/openvpn/pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ ifconfig_pool_test(in_addr_t start, in_addr_t end)
ifconfig_pool_handle h;
in_addr_t local, remote;
char buf[256];
openvpn_snprintf(buf, sizeof(buf), "common-name-%d", i);
snprintf(buf, sizeof(buf), "common-name-%d", i);
#ifdef DUP_CN
cn = NULL;
#else
Expand Down
Loading

0 comments on commit 130548f

Please sign in to comment.