Re: Incorrect usage of strtol, atoi for non-numeric junk inputs

On Thu, Jul 08, 2021 at 05:30:23PM +0900, Kyotaro Horiguchi wrote:
> Looked through the three threads.

Thanks!

> [1] is trying to expose pg_strtoint16/32 to frontend, but I don't see
> much point in doing that in conjunction with [2] or this thread. Since
> the integral parameter values of pg-commands are in int, which the
> exising function strtoint() is sufficient to read.  So even [2] itself
> doesn't need to utilize [1].

It sounds sensible from here to just use strtoint(), some strtol(),
son strtod() and call it a day as these are already available.

> -                    wait_seconds = atoi(optarg);
> +                    errno = 0;
> +                    wait_seconds = strtoint(optarg, &endptr, 10);
> +                    if (*endptr || errno == ERANGE || wait_seconds < 0)
> +                    {
> +                        pg_log_error("invalid timeout \"%s\"", optarg);
> +                        exit(1);
> +                    }
> [ ... ]
> -                killproc = atol(argv[++optind]);
> +                errno = 0;
> +                killproc = strtol(argv[++optind], &endptr, 10);
> +                if (*endptr || errno == ERANGE || killproc < 0)
> +                {
> +                    pg_log_error("invalid process ID \"%s\"", argv[optind]);
> +                    exit(1);
> +                }

Er, wait.  We've actually allowed negative values for pg_ctl
--timeout or the subcommand kill!?

>              case 'j':
> -                user_opts.jobs = atoi(optarg);
> +                errno = 0;
> +                user_opts.jobs = strtoint(optarg, &endptr, 10);
> +                /**/
> +                if (*endptr || errno == ERANGE)
> +                    pg_fatal("invalid number of jobs %s\n", optarg);
> +                    
>                  break;

This one in pg_upgrade is incomplete.  Perhaps the missing comment
should tell that negative job values are checked later on?
--
Michael