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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZAgNCCUMjP3W35qd@chenyu5-mobl1>
Date:   Wed, 8 Mar 2023 12:20:24 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
CC:     "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Len Brown <lenb@...nel.org>,
        Robert Moore <robert.moore@...el.com>,
        <linux-acpi@...r.kernel.org>,
        <acpica-devel@...ts.linuxfoundation.org>,
        <linux-kernel@...r.kernel.org>,
        Hariganesh Govindarajulu <hariganesh.govindarajulu@...el.com>
Subject: Re: [PATCH] ACPI: tools: pfrut: Check if the input of level and type
 is in the right numeric range

Hi Rafael,
thanks for the review,
On 2023-03-07 at 12:38:02 +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 7, 2023 at 4:46 AM Chen Yu <yu.c.chen@...el.com> 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?
> 
Yes.
> >  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@...el.com>
> > Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> > ---
> >  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?
> 
Yes, strtol() would be simpler:
c = strtol(optarg, &endptr, 0)
if (*endptr)
	exit(1);
I'll change it in next version.

thanks,
Chenyu