| Message ID | 20180615013156.2698-1-duncan_roe@optusnet.com.au |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | extensions: ebt_string: take action if snprintf discards data | expand |
Hi Duncan, On Fri, Jun 15, 2018 at 11:31:56AM +1000, Duncan Roe wrote: > commit e6359eedfbf497e52d52451072aea4713ed80a88 eliminated a gcc warning that > strncpy could make a string w/out a NUL terminator. This commit refers to header update, is this the right commit you're refering to or probably a different one should come here instead? commit e6359eedfbf497e52d52451072aea4713ed80a88 [...] build: update ebtables.h from kernel and drop local unused copy > snprintf guarantees NUL-termination (so fixes that possibility). > But, snprintf may discard data to make room for the NUL. > This patch errors straight away in that eventuality. > > Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> > --- > extensions/ebt_string.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c > index 3deff1b..79e24dc 100644 > --- a/extensions/ebt_string.c > +++ b/extensions/ebt_string.c > @@ -168,7 +168,9 @@ static int parse(int c, char **argv, int argc, const struct ebt_u_entry *entry, > ebt_check_option2(flags, OPT_STRING_ALGO); > if (ebt_check_inverse2(optarg)) > ebt_print_error2("Unexpected `!' after --string-algo"); > - snprintf(info->algo, sizeof(info->algo), "%s", optarg); > + if (snprintf(info->algo, sizeof(info->algo), "%s", optarg) >= > + sizeof(info->algo)) > + ebt_print_error2("\"%s\" is truncated", info->algo); > break; > case STRING_ICASE: > ebt_check_option2(flags, OPT_STRING_ICASE); > -- > 2.9.0 > > -- > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pablo, On Wed, Jun 27, 2018 at 05:28:48PM +0200, Pablo Neira Ayuso wrote: > Hi Duncan, > > On Fri, Jun 15, 2018 at 11:31:56AM +1000, Duncan Roe wrote: > > commit e6359eedfbf497e52d52451072aea4713ed80a88 eliminated a gcc warning that > > strncpy could make a string w/out a NUL terminator. > > This commit refers to header update, is this the right commit you're > refering to or probably a different one should come here instead? Sorry, my bad. I meant the next commit, 56993546c80576986930f9bae7ae4ba744b1e508 extensions: fix build failure on fc28 > > commit e6359eedfbf497e52d52451072aea4713ed80a88 > [...] > > build: update ebtables.h from kernel and drop local unused copy > > > snprintf guarantees NUL-termination (so fixes that possibility). > > But, snprintf may discard data to make room for the NUL. > > This patch errors straight away in that eventuality. > > > > Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> > > --- > > extensions/ebt_string.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c > > index 3deff1b..79e24dc 100644 > > --- a/extensions/ebt_string.c > > +++ b/extensions/ebt_string.c > > @@ -168,7 +168,9 @@ static int parse(int c, char **argv, int argc, const struct ebt_u_entry *entry, > > ebt_check_option2(flags, OPT_STRING_ALGO); > > if (ebt_check_inverse2(optarg)) > > ebt_print_error2("Unexpected `!' after --string-algo"); > > - snprintf(info->algo, sizeof(info->algo), "%s", optarg); > > + if (snprintf(info->algo, sizeof(info->algo), "%s", optarg) >= > > + sizeof(info->algo)) > > + ebt_print_error2("\"%s\" is truncated", info->algo); > > break; > > case STRING_ICASE: > > ebt_check_option2(flags, OPT_STRING_ICASE); > > -- > > 2.9.0 Can you fix or would you prefer I re-submit? I guess this has taught me to include the summary line along with the patch id in future :/ Cheers ... Duncan. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 28, 2018 at 07:57:32AM +1000, Duncan Roe wrote: [...] > Sorry, my bad. I meant the next commit, 56993546c80576986930f9bae7ae4ba744b1e508 > extensions: fix build failure on fc28 [...] > Can you fix or would you prefer I re-submit? Just fixed it before applying, no problem, thanks for clarifying. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c index 3deff1b..79e24dc 100644 --- a/extensions/ebt_string.c +++ b/extensions/ebt_string.c @@ -168,7 +168,9 @@ static int parse(int c, char **argv, int argc, const struct ebt_u_entry *entry, ebt_check_option2(flags, OPT_STRING_ALGO); if (ebt_check_inverse2(optarg)) ebt_print_error2("Unexpected `!' after --string-algo"); - snprintf(info->algo, sizeof(info->algo), "%s", optarg); + if (snprintf(info->algo, sizeof(info->algo), "%s", optarg) >= + sizeof(info->algo)) + ebt_print_error2("\"%s\" is truncated", info->algo); break; case STRING_ICASE: ebt_check_option2(flags, OPT_STRING_ICASE);
commit e6359eedfbf497e52d52451072aea4713ed80a88 eliminated a gcc warning that strncpy could make a string w/out a NUL terminator. snprintf guarantees NUL-termination (so fixes that possibility). But, snprintf may discard data to make room for the NUL. This patch errors straight away in that eventuality. Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> --- extensions/ebt_string.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)