From 4b913748e413314b69c315c314c3d07e10471712 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Mon, 13 Jul 2020 13:43:03 +0200 Subject: [PATCH 1/2] utils: correctly remove the whole line if `str` does not point to its beginning The following scenario triggers a bug when the line is not removed completely and the two lines are merged instead. ``` BEGINNING {if "condition":true|false} END {include if "condition"} NEXT LINE -> BEGINNING falseNEXT LINE ``` This is because `match_string` points after the first condition and we only remove the line to this point. Therefore we need to interate before `match_string` so we can find the real line start. Resolves: https://github.com/authselect/authselect/issues/218 --- src/lib/files/system.c | 2 +- src/lib/util/string.c | 10 ++-- src/lib/util/string.h | 3 +- src/lib/util/template.c | 16 ++++--- src/tests/test_util_template.c | 84 ++++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/src/lib/files/system.c b/src/lib/files/system.c index 453e2f4d6fc42ffd2e9bdfc773a38972802c1cb2..ef354b7583914cec07ff3c017394daa5b6cc27e1 100644 --- a/src/lib/files/system.c +++ b/src/lib/files/system.c @@ -178,7 +178,7 @@ authselect_system_nsswitch_delete_maps(char **maps, map_len = m[1].rm_eo - m[1].rm_so; for (i = 0; maps[i] != NULL; i++) { if (strncmp(map_name, maps[i], map_len) == 0) { - string_remove_line(match_string, m[1].rm_so); + string_remove_line(content, match_string, m[1].rm_so); break; } } diff --git a/src/lib/util/string.c b/src/lib/util/string.c index 0f3936681c6c8af1be940f92a21dfc15dafe4e42..e53a81c250711e5caab8e5f4f751371c332e6b92 100644 --- a/src/lib/util/string.c +++ b/src/lib/util/string.c @@ -272,17 +272,21 @@ string_replace_position(char *str, size_t start, size_t end, const char *with) } void -string_remove_line(char *str, size_t inner_position) +string_remove_line(char *beginning, char *str, size_t inner_position) { char *left; - for (left = str + inner_position; left != str; left--) { + /* str may not be the beginning of the line so we need to refer + * to iterate until we reach the beginning */ + for (left = str + inner_position; left != beginning; left--) { if (*(left - 1) == '\n') { break; } } - for (; *left != '\0'; left++) { + /* Remove the whole line that is in front of our string and then iterate + * to the line end or string end. */ + for (; left < str || *left != '\0'; left++) { if (*left == '\n') { *left = '\0'; break; diff --git a/src/lib/util/string.h b/src/lib/util/string.h index e550d853d3fa0699909b84cc9febdae9d5884b9f..724460e771389ac3c015806111d6052ffbfa7566 100644 --- a/src/lib/util/string.h +++ b/src/lib/util/string.h @@ -142,11 +142,12 @@ string_replace_position(char *str, size_t start, size_t end, const char *with); * When all replacements are done, call @string_replace_shake() to create * the final string. * + * @param beginning Pointer to the left most character of the string. * @param str Destination string. * @param inner_position Position inside the line the will be removed. */ void -string_remove_line(char *str, size_t inner_position); +string_remove_line(char *beginning, char *str, size_t inner_position); /** * Remove string from @from (including) to @to (excluding). diff --git a/src/lib/util/template.c b/src/lib/util/template.c index f86a26a8344f1c140861f1572b74614604624dd5..12324aa9c16b500f481739a46652f65f98863fed 100644 --- a/src/lib/util/template.c +++ b/src/lib/util/template.c @@ -211,6 +211,7 @@ template_match_get_values(const char *match_string, static errno_t template_match_replace(char ***features, + char *beginning, char *match_string, regmatch_t *match, enum template_operator op, @@ -231,7 +232,7 @@ template_match_replace(char ***features, switch (op) { case OP_CONTINUE: if (enabled) { - string_remove_line(match_string, match->rm_so); + string_remove_line(beginning, match_string, match->rm_so); break; } @@ -239,7 +240,7 @@ template_match_replace(char ***features, break; case OP_STOP: if (!enabled) { - string_remove_line(match_string, match->rm_so); + string_remove_line(beginning, match_string, match->rm_so); break; } @@ -251,7 +252,7 @@ template_match_replace(char ***features, break; } - string_remove_line(match_string, match->rm_so); + string_remove_line(beginning, match_string, match->rm_so); break; case OP_EXCLUDE: if (!enabled) { @@ -259,7 +260,7 @@ template_match_replace(char ***features, break; } - string_remove_line(match_string, match->rm_so); + string_remove_line(beginning, match_string, match->rm_so); break; case OP_IMPLY: if (enabled) { @@ -269,7 +270,7 @@ template_match_replace(char ***features, } } - string_remove_line(match_string, match->rm_so); + string_remove_line(beginning, match_string, match->rm_so); break; case OP_IF: replacement = enabled ? if_true : if_false; @@ -460,8 +461,9 @@ template_process_operators(const char **features, goto done; } - ret = template_match_replace(&features_copy, match_string, &m[0], op, - expression, if_true, if_false, value); + ret = template_match_replace(&features_copy, content, match_string, + &m[0], op, expression, + if_true, if_false, value); if (expression != NULL) { free(expression); diff --git a/src/tests/test_util_template.c b/src/tests/test_util_template.c index 90327ea68d0e09df98befde4835e90350f0c6238..fac3f4c94e3553c71ee538a5725fb0a734f89382 100644 --- a/src/tests/test_util_template.c +++ b/src/tests/test_util_template.c @@ -269,6 +269,86 @@ void test_template_imply_if(void **state) free(result); } +void test_template_if_and_include__true(void **state) +{ + const char *myfeatures[] = { + "true", + NULL + }; + + const char *template = + "L1 {if \"f1\":T1|T2} T3 {include if \"true\"} \n" + "L2 \n" + ""; + const char *expected = + "L1 T2 T3\n" + "L2\n" + ""; + + char *result = template_generate(template, myfeatures); + assert_string_equal(expected, result); + free(result); +} + +void test_template_if_and_include__false(void **state) +{ + const char *myfeatures[] = { + NULL + }; + + const char *template = + "L1 {if \"f1\":T1|T2} T3 {include if \"true\"} \n" + "L2 \n" + ""; + const char *expected = + "L2\n" + ""; + + char *result = template_generate(template, myfeatures); + assert_string_equal(expected, result); + free(result); +} + +void test_template_if_and_exclude__true(void **state) +{ + const char *myfeatures[] = { + "true", + NULL + }; + + const char *template = + "L1 {if \"f1\":T1|T2} T3 {exclude if \"true\"} \n" + "L2 \n" + ""; + const char *expected = + "L2\n" + ""; + + char *result = template_generate(template, myfeatures); + assert_string_equal(expected, result); + free(result); +} + +void test_template_if_and_exclude__false(void **state) +{ + const char *myfeatures[] = { + NULL + }; + + const char *template = + "L1 {if \"f1\":T1|T2} T3 {exclude if \"true\"} \n" + "L2 \n" + ""; + const char *expected = + "L1 T2 T3\n" + "L2\n" + ""; + + char *result = template_generate(template, myfeatures); + assert_string_equal(expected, result); + free(result); +} + int main(int argc, const char *argv[]) { @@ -281,6 +361,10 @@ int main(int argc, const char *argv[]) cmocka_unit_test(test_template_continue_if), cmocka_unit_test(test_template_list_features), cmocka_unit_test(test_template_imply_if), + cmocka_unit_test(test_template_if_and_include__true), + cmocka_unit_test(test_template_if_and_include__false), + cmocka_unit_test(test_template_if_and_exclude__true), + cmocka_unit_test(test_template_if_and_exclude__false), }; return cmocka_run_group_tests(tests, NULL, NULL); -- 2.25.4