Re: [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range

[Rafael J. Wysocki]

On Tue, Mar 7, 2023 at 4:46 AM Chen Yu <yu.c.chen@xxxxxxxxx> wrote:
>
> The user can provide arbitrary non-numeic value to level and type, which
> brings unexpected behavior:

So I guess the expected behavior would be to throw an error?

>  pfrut -h
> usage: pfrut [OPTIONS]
> code injection:
> -l, --load
> -s, --stage
> -a, --activate
> -u, --update [stage and activate]
> -q, --query
> -d, --revid
> update telemetry:
> -G, --getloginfo
> -T, --type(0:execution, 1:history)
> -L, --level(0, 1, 2, 4)
> -R, --read
> -D, --revid log
>
>  pfrut -T A
>  pfrut -G
> log_level:0
> log_type:0
> log_revid:2
> max_data_size:65536
> chunk1_size:0
> chunk2_size:1530
> rollover_cnt:0
> reset_cnt:17
>
> Fix this by restricting the input is in the expected range.
>
> Reported-by: Hariganesh Govindarajulu <hariganesh.govindarajulu@xxxxxxxxx>
> Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> ---
>  tools/power/acpi/tools/pfrut/pfrut.c | 33 ++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/tools/power/acpi/tools/pfrut/pfrut.c b/tools/power/acpi/tools/pfrut/pfrut.c
> index 52aa0351533c..ed672efef83b 100644
> --- a/tools/power/acpi/tools/pfrut/pfrut.c
> +++ b/tools/power/acpi/tools/pfrut/pfrut.c
> @@ -22,6 +22,7 @@
>  #include <sys/ioctl.h>
>  #include <sys/mman.h>
>  #include <uuid/uuid.h>
> +#include <ctype.h>
>  #include PFRUT_HEADER
>
>  char *capsule_name;
> @@ -77,6 +78,18 @@ static void help(void)
>                 progname);
>  }
>
> +static int is_digit_input(char *str)
> +{
> +       char *scan;
> +
> +       for (scan = str; *scan != '\0'; scan++) {
> +               if (!isdigit(*scan))
> +                       return 0;
> +       }
> +
> +       return 1;
> +}
> +
>  char *option_string = "l:sauqd:GT:L:RD:h";
>  static struct option long_options[] = {
>         {"load", required_argument, 0, 'l'},
> @@ -125,11 +138,31 @@ static void parse_options(int argc, char **argv)
>                         log_getinfo = 1;
>                         break;
>                 case 'T':
> +                       if (!is_digit_input(optarg)) {
> +                               printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
> +                               exit(1);
> +                       }
> +

Wouldn't using strtol() instead of atoi() work?

>                         log_type = atoi(optarg);
> +                       if (log_type != 0 && log_type != 1) {
> +                               printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
> +                               exit(1);
> +                       }
> +
>                         set_log_type = 1;
>                         break;
>                 case 'L':
> +                       if (!is_digit_input(optarg)) {
> +                               printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
> +                               exit(1);
> +                       }
> +
>                         log_level = atoi(optarg);
> +                       if (log_level > 4) {
> +                               printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
> +                               exit(1);
> +                       }
> +
>                         set_log_level = 1;
>                         break;
>                 case 'R':
> --
> 2.25.1
>