Re: [PATCH ipset 3/4] Check setname length in session code before copying it

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

 



On Wed, 22 Aug 2018, Stefano Brivio wrote:

> We might overrun the buffer used to save it otherwise.
> 
> Signed-off-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
> ---
>  lib/session.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index ca96aaa57ea6..7cf3858ca97d 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -1069,6 +1069,7 @@ callback_list(struct ipset_session *session, struct nlattr *nla[],
>  
>  	if (nla[IPSET_ATTR_DATA] != NULL) {
>  		struct nlattr *cattr[IPSET_ATTR_CREATE_MAX+1] = {};
> +		const char *setname;
>  
>  		if (!(nla[IPSET_ATTR_TYPENAME] &&
>  		      nla[IPSET_ATTR_FAMILY] &&
> @@ -1097,7 +1098,12 @@ callback_list(struct ipset_session *session, struct nlattr *nla[],
>  				cmd2name[cmd]);
>  		if (list_create(session, cattr) != MNL_CB_OK)
>  			return MNL_CB_ERROR;
> -		strcpy(session->saved_setname, ipset_data_setname(data));
> +		setname = ipset_data_setname(data);
> +		if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> +			FAILURE("Broken %s kernel message: "
> +				"setname missing or too long!",
> +				cmd2name[cmd]);
> +		strcpy(session->saved_setname, setname);
>  	}
>  
>  	if (nla[IPSET_ATTR_ADT] != NULL) {
> @@ -2014,7 +2020,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);
>  		ipset_data_reset(data);
>  		/* Don't commit: we may aggregate next command */
>  		ret = 0;

The data received via netlink is verified in attr2data() (wrapped in 
ATTR2DATA). The verification of setname length should have been done 
there. So please correct attr2data() instead.

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