Fix and improve get_name_value() and unit tests

Fix possible segfault if name has leading white space
Pass the name param to get_name_value() as char *, so it cannot be
modified ever

Improve unit tests for get_name_value and proxyspec_parse
pull/48/head
Soner Tari 3 years ago
parent ab7674b652
commit d877b9a635

@ -4515,32 +4515,36 @@ set_proxyspec_option(proxyspec_t *spec, const char *argv0,
/*
* Separator param is needed for command line options only.
* Conf file option separator is ' '.
* Do not modify the name param, the caller may try to free it causing signal 6 crash.
* Conf file option separator is ' ', on the command line is '='.
* Allows multiple separators between name and value.
*/
int
get_name_value(char **name, char **value, const char sep, int line_num)
get_name_value(char *name, char **value, const char sep, int line_num)
{
size_t len = strlen(*name);
size_t len = strlen(name);
// Find end of name and null-terminate
char *n = *name;
while (*n != ' ' && *n != '\t' && *n != '\r' && *n != '\n' && *n != sep)
char *n = name;
while (*n != '\0' && *n != ' ' && *n != '\t' && *n != '\r' && *n != '\n' && *n != sep)
n++;
*n = '\0';
if (!*name) {
size_t name_len = strlen(name);
if (!name_len) {
fprintf(stderr, "Error in option: No option name on line %d\n", line_num);
// Return empty value
*value = name;
return -1;
}
size_t name_len = strlen(*name);
if (len == name_len) {
fprintf(stderr, "Error in option: No option value on line %d\n", line_num);
#ifdef DEBUG_OPTS
log_dbg_printf("Warning in option: No option separator on line %d\n", line_num);
#endif /* DEBUG_OPTS */
// Return empty value
*value = *name + name_len;
return -1;
*value = name + name_len;
return 0;
}
// Trim left of value (skip white space and sep until value)
@ -4549,8 +4553,17 @@ get_name_value(char **name, char **value, const char sep, int line_num)
*value = n;
size_t value_len = strlen(*value);
if (!value_len) {
#ifdef DEBUG_OPTS
log_dbg_printf("Warning in option: No option value on line %d\n", line_num);
#endif /* DEBUG_OPTS */
return 0;
}
// Trim right of value
n = *value + strlen(*value) - 1;
n = *value + value_len - 1;
while (*n == ' ' || *n == '\t' || *n == '\r' || *n == '\n' || *n == sep)
n--;
*(n + 1) = '\0';
@ -4631,7 +4644,7 @@ load_proxyspec_struct(global_t *global, const char *argv0, char **natengine, int
continue;
}
retval = get_name_value(&name, &value, ' ', *line_num);
retval = get_name_value(name, &value, ' ', *line_num);
if (retval == 0) {
retval = set_proxyspec_option(spec, argv0, name, value, natengine, spec_addrs, *line_num);
}
@ -4871,7 +4884,7 @@ global_set_option(global_t *global, const char *argv0, const char *optarg,
for (name = line; *name == ' ' || *name == '\t'; name++);
/* Command line option separator is '=' */
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
if (retval == 0) {
/* Line number param is for conf file, pass 0 for command line options */
int line_num = 0;
@ -4923,7 +4936,7 @@ opts_load_conffile(global_t *global, const char *argv0, char *conffile, char **n
continue;
}
retval = get_name_value(&name, &value, ' ', line_num);
retval = get_name_value(name, &value, ' ', line_num);
if (retval == 0) {
retval = set_global_option(global, argv0, name, value, natengine, &line_num, f, tmp_global_opts);
}

@ -457,7 +457,7 @@ int global_set_debug_level(const char *) NONNULL(1) WUNRES;
void global_set_statslog(global_t *) NONNULL(1);
int is_yesno(const char *) WUNRES;
int get_name_value(char **, char **, const char, int) WUNRES;
int get_name_value(char *, char **, const char, int) WUNRES;
int global_set_option(global_t *, const char *, const char *, char **, tmp_global_opts_t *) NONNULL(1,2,3,5) WUNRES;
int global_set_leafkey(global_t *, const char *, const char *) NONNULL(1,2,3) WUNRES;
int global_set_leafcertdir(global_t *, const char *, const char *) NONNULL(1,2,3) WUNRES;

@ -163,34 +163,42 @@ START_TEST(proxyspec_parse_03)
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
// Disable error messages
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
fail_unless(rv == -1, "failed to reject spec");
START_TEST(proxyspec_parse_04)
{
global_t *global = global_new();
int argc = 5;
char **argv = argv01;
argc = 5;
rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
fail_unless(rv == -1, "failed to reject spec");
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
argc = 5;
argv = argv07;
rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
fail_unless(rv == -1, "failed to reject spec");
argc = 5;
argv = argv06;
rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
fail_unless(rv == -1, "failed to reject spec");
argc = 5;
argv = argv08;
rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
fail_unless(rv == -1, "failed to reject spec");
argc = 6;
argv = argv13;
rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
fail_unless(rv == -1, "failed to reject spec");
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
START_TEST(proxyspec_parse_05)
START_TEST(proxyspec_parse_04)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -220,7 +228,7 @@ START_TEST(proxyspec_parse_05)
}
END_TEST
START_TEST(proxyspec_parse_06)
START_TEST(proxyspec_parse_05)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -250,7 +258,7 @@ START_TEST(proxyspec_parse_06)
}
END_TEST
START_TEST(proxyspec_parse_07)
START_TEST(proxyspec_parse_06)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -280,7 +288,7 @@ START_TEST(proxyspec_parse_07)
}
END_TEST
START_TEST(proxyspec_parse_08)
START_TEST(proxyspec_parse_07)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -309,43 +317,7 @@ START_TEST(proxyspec_parse_08)
}
END_TEST
START_TEST(proxyspec_parse_09)
{
global_t *global = global_new();
int argc = 5;
char **argv = argv07;
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
START_TEST(proxyspec_parse_10)
{
global_t *global = global_new();
int argc = 5;
char **argv = argv06;
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
START_TEST(proxyspec_parse_11)
START_TEST(proxyspec_parse_08)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -375,26 +347,8 @@ START_TEST(proxyspec_parse_11)
}
END_TEST
START_TEST(proxyspec_parse_12)
{
global_t *global = global_new();
int argc = 5;
char **argv = argv08;
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
#ifndef TRAVIS
START_TEST(proxyspec_parse_13)
START_TEST(proxyspec_parse_09)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -435,7 +389,7 @@ START_TEST(proxyspec_parse_13)
}
END_TEST
START_TEST(proxyspec_parse_14)
START_TEST(proxyspec_parse_10)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -478,7 +432,7 @@ START_TEST(proxyspec_parse_14)
END_TEST
#endif /* !TRAVIS */
START_TEST(proxyspec_parse_15)
START_TEST(proxyspec_parse_11)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -507,7 +461,7 @@ START_TEST(proxyspec_parse_15)
}
END_TEST
START_TEST(proxyspec_parse_16)
START_TEST(proxyspec_parse_12)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -548,25 +502,7 @@ START_TEST(proxyspec_parse_16)
}
END_TEST
START_TEST(proxyspec_parse_17)
{
global_t *global = global_new();
int argc = 6;
char **argv = argv13;
tmp_global_opts_t *tmp_global_opts = malloc(sizeof(tmp_global_opts_t));
memset(tmp_global_opts, 0, sizeof(tmp_global_opts_t));
close(2);
int rv = proxyspec_parse(&argc, &argv, NATENGINE, global, "sslproxy", tmp_global_opts);
global_free(global);
tmp_global_opts_free(tmp_global_opts);
if (rv == -1)
exit(EXIT_FAILURE);
}
END_TEST
START_TEST(proxyspec_parse_18)
START_TEST(proxyspec_parse_13)
{
global_t *global = global_new();
proxyspec_t *spec = NULL;
@ -913,100 +849,127 @@ START_TEST(opts_get_name_value_01)
char *value;
name = strdup("Name Value");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
name = strdup("Name Value");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
// Leading white space must be handled by the caller,
// so we don't have a test for " Name Value", or similar
// Leading white space must be handled by the caller
// We cannot modify the name pointer, so we return -1
// So we don't actually need a test for " Name Value", or similar
name = strdup(" Name Value");
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, ""), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == -1, "failed rejecting leading white space, empty name");
free(name);
name = strdup("Name Value ");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
name = strdup("Name=Value");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
// Leading white space must be handled by the caller
// We cannot modify the name pointer, so we return -1
// So we don't actually need a test for " Name Value", or similar
name = strdup(" Name=Value");
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, ""), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == -1, "failed rejecting leading white space, empty name");
free(name);
name = strdup("Name=Value ");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
name = strdup("Name = Value");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
name = strdup("Name = Value ");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, "Value"), "failed parsing value");
fail_unless(retval == 0, "failed parsing name value");
free(name);
// Name without value, e.g. '}' char is used for marking the end of structured proxyspecs
// so do not reject any form of just name, return success
name = strdup("Name");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(retval == -1, "failed rejecting just name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing just name");
free(name);
name = strdup("Name ");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name ");
retval = get_name_value(&name, &value, ' ', 0);
retval = get_name_value(name, &value, ' ', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name");
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name=");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name= ");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name =");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
free(name);
name = strdup("Name = ");
retval = get_name_value(&name, &value, '=', 0);
retval = get_name_value(name, &value, '=', 0);
fail_unless(!strcmp(name, "Name"), "failed parsing name");
fail_unless(!strcmp(value, ""), "failed parsing value");
fail_unless(retval == 0, "failed parsing name empty value");
@ -1026,24 +989,19 @@ opts_suite(void)
#ifndef TRAVIS
tcase_add_test(tc, proxyspec_parse_02); /* IPv6 */
#endif /* !TRAVIS */
tcase_add_exit_test(tc, proxyspec_parse_03, EXIT_FAILURE);
tcase_add_exit_test(tc, proxyspec_parse_04, EXIT_FAILURE);
tcase_add_test(tc, proxyspec_parse_03);
tcase_add_test(tc, proxyspec_parse_04);
tcase_add_test(tc, proxyspec_parse_05);
tcase_add_test(tc, proxyspec_parse_06);
tcase_add_test(tc, proxyspec_parse_07);
tcase_add_test(tc, proxyspec_parse_08);
tcase_add_exit_test(tc, proxyspec_parse_09, EXIT_FAILURE);
tcase_add_exit_test(tc, proxyspec_parse_10, EXIT_FAILURE);
tcase_add_test(tc, proxyspec_parse_11);
tcase_add_exit_test(tc, proxyspec_parse_12, EXIT_FAILURE);
#ifndef TRAVIS
tcase_add_test(tc, proxyspec_parse_13); /* IPv6 */
tcase_add_test(tc, proxyspec_parse_14); /* IPv6 */
tcase_add_test(tc, proxyspec_parse_09); /* IPv6 */
tcase_add_test(tc, proxyspec_parse_10); /* IPv6 */
#endif /* !TRAVIS */
tcase_add_test(tc, proxyspec_parse_15);
tcase_add_test(tc, proxyspec_parse_16);
tcase_add_exit_test(tc, proxyspec_parse_17, EXIT_FAILURE);
tcase_add_test(tc, proxyspec_parse_18);
tcase_add_test(tc, proxyspec_parse_11);
tcase_add_test(tc, proxyspec_parse_12);
tcase_add_test(tc, proxyspec_parse_13);
tcase_add_test(tc, proxyspec_set_proto_01);
suite_add_tcase(s, tc);

Loading…
Cancel
Save