From 670c7cb1969d2bd1bf96753a6f9277a88d8ad642 Mon Sep 17 00:00:00 2001 From: Eric Richter Date: Wed, 3 Jan 2024 14:32:11 -0600 Subject: [PATCH] guest/generate: fix multiple input/output format specifier argument parsing Currently, it is ASSUMED but not enforced that the first argument to `secvarctl generate` is a format specifier in the form `a:b`, where a,b correspond to the formats to be generated/converted. This argument parse logic is applied to ANY bare/positional arguments applied to `generate`, including repeated ones. Therefore, any of the following cases can occur: 1. multiple format specifiers can be supplied: `secvarctl generate a:b b:c c:d -n NAME -c file.crt` 2. invalid/erroneous arguments can be mis-interpreted as a format specifier: `secvarctl generate a:b -n NAME -c file.crt -a 0`, where `-a` does not take in an argument, thus yielding "invalid format" 3. format specifiers can be ANYWHERE in the arg list: `secvarctl generate -n NAME -c file.crt a:b` This commit addresses case 1 and 2 by only allowing format specifiers to be declared ONCE in an argument list. Furthermore, the format specifier is validated while parsing, so in the event the erroneous positional argument is read in first, the parser will immediately complain. Case 3 is NOT addressed by this commit, however is a consideration for future refactoring of the argument parsing logic. Signed-off-by: Eric Richter --- backends/guest/guest_svc_generate.c | 25 +++++++++++++++++++++++-- test/guest_tests.py | 17 +++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/backends/guest/guest_svc_generate.c b/backends/guest/guest_svc_generate.c index d92b4ecb..be7538ba 100644 --- a/backends/guest/guest_svc_generate.c +++ b/backends/guest/guest_svc_generate.c @@ -483,6 +483,19 @@ static int parse_options(int key, char *arg, struct argp_state *state) args->append_flag = 1; break; case ARGP_KEY_ARG: + /* there should only be one format specifier, error if another is supplied */ + if (args->input_form && args->output_form) { + prlog(PR_ERR, "ERROR: unknown additional positional argument %s\n", arg); + rc = ARG_PARSE_FAIL; + break; + } + /* both forms should be either set or NULL, this should never be reached. */ + if (!args->input_form ^ !args->output_form) { + prlog(PR_ERR, + "ERROR: only one of input_form/output_form is set, this should not happen\n"); + rc = ARG_PARSE_FAIL; + break; + } /* check if reset key is desired */ if (!strcmp(arg, "reset")) { args->input_form = "reset"; @@ -493,14 +506,22 @@ static int parse_options(int key, char *arg, struct argp_state *state) /* else set input and output formats */ args->input_form = strtok(arg, ":"); args->output_form = strtok(NULL, ":"); + + /* verify both input and output forms are parsed correctly, error otherwise */ + if (!args->input_form || !args->output_form) { + prlog(PR_ERR, + "ERROR: '%s' is not in the correct ':' form, see usage...\n", + arg); + rc = ARG_PARSE_FAIL; + } break; case ARGP_KEY_SUCCESS: /* check that all essential args are given and valid */ if (args->help_flag) break; else if (args->input_form == NULL || args->output_form == NULL) - prlog(PR_ERR, "ERROR: incorrect ':', see " - "usage...\n"); + prlog(PR_ERR, + "ERROR: invalid or missing ':', see usage...\n"); else if (args->time && validate_time(args->time)) prlog(PR_ERR, "invalid timestamp flag '-t YYYY-MM-DDThh:mm:ss' , " "see usage...\n"); diff --git a/test/guest_tests.py b/test/guest_tests.py index 32883809..c0e5e782 100644 --- a/test/guest_tests.py +++ b/test/guest_tests.py @@ -172,6 +172,23 @@ def setUp(self): # f.write(f"POWER SECVAR LOCATION( {SECVARPATH} ) DOES NOT EXIST SO NO TESTS RAN\n") # f.close() + def test_malformed_generate(self): + cert = cert_files[0] # arbitrarily use the first cert for testing + + # Generate without a inform:outform should fail + cmd = list(filter(lambda x: x, generate_esl("db", "", cert, "foo.esl"))) + self.assertCmdFalse(cmd) + + # Generate with bad inform:output should fail + cmd.append("beans") + self.assertCmdFalse(cmd) + cmd.pop(-1) + + # Generate with more than one inform:outform should also fail + cmd.append("c:e") + cmd.append("c:e") + self.assertCmdFalse(cmd) + def test_generate_esl_files(self): for var_name in variables: esl_file = gen_dir + var_name + ".esl"