Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: usr.bin/patch: use strtonum instead of atoi()
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
Omar Polo <op@omarpolo.com>, tech@openbsd.org
Date:
Thu, 29 Aug 2024 11:00:14 -0600

Download raw body.

Thread
The bounds are pretty high, but it looks reasonable.

Alexander Bluhm <bluhm@openbsd.org> wrote:

> On Thu, Aug 29, 2024 at 12:42:18AM +0200, Omar Polo wrote:
> > atoi() is only used to parse numbers from arguments, so there's no
> > trailing garbage.  As a bonus, we reject negative values which makes no
> > sense.  INT_MAX is just an upper limit.  way too big.
> > 
> > oks?
> 
> s/fuzz factor/maximum fuzz/
> s/debug value/debug number/
> 
> This is closer to the wording in the man page.
> 
> OK bluhm@
> 
> > diff 5a1af3cffe24ad9d9a983a89b5a57b51a5cc18b3 b30480ea2d6abd9e55cb8ea416d02bb6060dd225
> > commit - 5a1af3cffe24ad9d9a983a89b5a57b51a5cc18b3
> > commit + b30480ea2d6abd9e55cb8ea416d02bb6060dd225
> > blob - 96da0572381e10ed35a0196c6425fc0a5ec4f0c6
> > blob + 30f164cf922efc833a4fa48cdf1f847f977744ff
> > --- usr.bin/patch/patch.c
> > +++ usr.bin/patch/patch.c
> > @@ -542,6 +542,7 @@ get_some_switches(void)
> >  		{NULL,			0,			0,	0}
> >  	};
> >  	int ch;
> > +	const char *errstr;
> >  
> >  	rejname[0] = '\0';
> >  	Argc_last = Argc;
> > @@ -598,7 +599,10 @@ get_some_switches(void)
> >  			force = true;
> >  			break;
> >  		case 'F':
> > -			maxfuzz = atoi(optarg);
> > +			maxfuzz = strtonum(optarg, 0, INT_MAX, &errstr);
> > +			if (errstr != NULL)
> > +				fatal("fuzz factor is %s: %s\n",
> > +				    errstr, optarg);
> >  			break;
> >  		case 'i':
> >  			if (++filec == MAXFILEC)
> > @@ -618,7 +622,10 @@ get_some_switches(void)
> >  			outname = xstrdup(optarg);
> >  			break;
> >  		case 'p':
> > -			strippath = atoi(optarg);
> > +			strippath = strtonum(optarg, 0, INT_MAX, &errstr);
> > +			if (errstr != NULL)
> > +				fatal("strip count is %s: %s\n",
> > +				    errstr, optarg);
> >  			break;
> >  		case 'r':
> >  			if (strlcpy(rejname, optarg,
> > @@ -647,7 +654,10 @@ get_some_switches(void)
> >  			break;
> >  #ifdef DEBUGGING
> >  		case 'x':
> > -			debug = atoi(optarg);
> > +			debug = strtonum(optarg, 0, INT_MAX, &errstr);
> > +			if (errstr != NULL)
> > +				fatal("debug value is %s: %s\n",
> > +				    errstr, optarg);
> >  			break;
> >  #endif
> >  		default:
>