Re: [PATCH ipset v2] Check setname length in session code before copying it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stefano,

On Thu, 30 Aug 2018, Stefano Brivio wrote:

> > > @@ -2014,7 +2021,11 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
> > >  	if (session->lineno != 0 &&
> > >  	    (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)) {
> > >  		/* Save setname for the next possible aggregated restore line */
> > > -		strcpy(session->saved_setname, ipset_data_setname(data));
> > > +		const char *setname = ipset_data_setname(data);
> > > +		if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> > > +			return ipset_err(session,
> > > +				"Invalid command: setname missing or too long");
> > > +		strcpy(session->saved_setname, setname);  
> > 
> > I don't understand this part: it is from userspace to kernel and ADD/DEL 
> > commands already verified that setname is filled out, in build_msg().
> 
> Right, I don't need to check that setname is there. Still, its length is 
> not checked as far as I can see. Would you suggest that I move the 
> length check to build_msg()?

build_msg() gets all the data for the netlink message from the ipset_data 
structure. The structure is filled up by parsers and the parsers verify 
the input, in this case ipset_parse_setname(). So the checking is 
unnecessary here.

Best regards,
Jozsef
-
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux