* [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser
@ 2024-10-12 22:00 Pablo Neira Ayuso
2024-10-12 22:00 ` [PATCH conntrack,v2 2/2] conntrack: improve --mark parser Pablo Neira Ayuso
2024-10-21 19:10 ` [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Jeremy Sowden
0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-12 22:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: jeremy
strtoul() is called with no error checking at all, add a helper
function to validate input is correct for values less than
UINT32_MAX.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: - remove value == 0 && errno == ERANGE check
- add assert to remember this only supports max up to UINT32_MAX
src/conntrack.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/src/conntrack.c b/src/conntrack.c
index 9fa49869b553..18829dbf79bc 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1213,6 +1213,26 @@ parse_parameter_mask(const char *arg, unsigned int *status, unsigned int *mask,
exit_error(PARAMETER_PROBLEM, "Bad parameter `%s'", arg);
}
+static int parse_value(const char *str, uint32_t *ret, uint64_t max)
+{
+ char *endptr;
+ uint64_t val;
+
+ assert(max <= UINT32_MAX);
+
+ errno = 0;
+ val = strtoul(str, &endptr, 0);
+ if (endptr == str ||
+ *endptr != '\0' ||
+ (val == ULONG_MAX && errno == ERANGE) ||
+ val > max)
+ return -1;
+
+ *ret = val;
+
+ return 0;
+}
+
static void
parse_u32_mask(const char *arg, struct u32_mask *m)
{
@@ -2918,6 +2938,7 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
struct ct_tmpl *tmpl;
int res = 0, partial;
union ct_address ad;
+ uint32_t value;
int c, cmd;
/* we release these objects in the exit_error() path. */
@@ -3078,17 +3099,19 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
case 'w':
case '(':
case ')':
+ if (parse_value(optarg, &value, UINT16_MAX) < 0)
+ exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
+
options |= opt2type[c];
- nfct_set_attr_u16(tmpl->ct,
- opt2attr[c],
- strtoul(optarg, NULL, 0));
+ nfct_set_attr_u16(tmpl->ct, opt2attr[c], value);
break;
case 'i':
case 'c':
+ if (parse_value(optarg, &value, UINT32_MAX) < 0)
+ exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
+
options |= opt2type[c];
- nfct_set_attr_u32(tmpl->ct,
- opt2attr[c],
- strtoul(optarg, NULL, 0));
+ nfct_set_attr_u32(tmpl->ct, opt2attr[c], value);
break;
case 'm':
options |= opt2type[c];
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH conntrack,v2 2/2] conntrack: improve --mark parser
2024-10-12 22:00 [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Pablo Neira Ayuso
@ 2024-10-12 22:00 ` Pablo Neira Ayuso
2024-10-21 19:10 ` Jeremy Sowden
2024-10-21 19:10 ` [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Jeremy Sowden
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-12 22:00 UTC (permalink / raw)
To: netfilter-devel; +Cc: jeremy
Enhance helper function to parse mark and mask (if available), bail out
if input is not correct.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: - remove value == 0 && errno == ERANGE check
src/conntrack.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/conntrack.c b/src/conntrack.c
index 18829dbf79bc..5bd966cad657 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1233,17 +1233,35 @@ static int parse_value(const char *str, uint32_t *ret, uint64_t max)
return 0;
}
-static void
+static int
parse_u32_mask(const char *arg, struct u32_mask *m)
{
- char *end;
+ uint64_t val, mask;
+ char *endptr;
+
+ val = strtoul(arg, &endptr, 0);
+ if (endptr == arg ||
+ (*endptr != '\0' && *endptr != '/') ||
+ (val == ULONG_MAX && errno == ERANGE) ||
+ val > UINT32_MAX)
+ return -1;
- m->value = (uint32_t) strtoul(arg, &end, 0);
+ m->value = val;
- if (*end == '/')
- m->mask = (uint32_t) strtoul(end+1, NULL, 0);
- else
+ if (*endptr == '/') {
+ mask = (uint32_t) strtoul(endptr + 1, &endptr, 0);
+ if (endptr == arg ||
+ *endptr != '\0' ||
+ (val == ULONG_MAX && errno == ERANGE) ||
+ val > UINT32_MAX)
+ return -1;
+
+ m->mask = mask;
+ } else {
m->mask = ~0;
+ }
+
+ return 0;
}
static int
@@ -3115,7 +3133,9 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
break;
case 'm':
options |= opt2type[c];
- parse_u32_mask(optarg, &tmpl->mark);
+ if (parse_u32_mask(optarg, &tmpl->mark) < 0)
+ exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
+
tmpl->filter_mark_kernel.val = tmpl->mark.value;
tmpl->filter_mark_kernel.mask = tmpl->mark.mask;
tmpl->filter_mark_kernel_set = true;
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH conntrack,v2 2/2] conntrack: improve --mark parser
2024-10-12 22:00 ` [PATCH conntrack,v2 2/2] conntrack: improve --mark parser Pablo Neira Ayuso
@ 2024-10-21 19:10 ` Jeremy Sowden
2024-10-22 9:49 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Sowden @ 2024-10-21 19:10 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]
On 2024-10-13, at 00:00:30 +0200, Pablo Neira Ayuso wrote:
> Enhance helper function to parse mark and mask (if available), bail out
> if input is not correct.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> v2: - remove value == 0 && errno == ERANGE check
>
> src/conntrack.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/src/conntrack.c b/src/conntrack.c
> index 18829dbf79bc..5bd966cad657 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -1233,17 +1233,35 @@ static int parse_value(const char *str, uint32_t *ret, uint64_t max)
> return 0;
> }
>
> -static void
> +static int
> parse_u32_mask(const char *arg, struct u32_mask *m)
> {
> - char *end;
> + uint64_t val, mask;
> + char *endptr;
> +
> + val = strtoul(arg, &endptr, 0);
> + if (endptr == arg ||
> + (*endptr != '\0' && *endptr != '/') ||
> + (val == ULONG_MAX && errno == ERANGE) ||
> + val > UINT32_MAX)
> + return -1;
>
> - m->value = (uint32_t) strtoul(arg, &end, 0);
> + m->value = val;
>
> - if (*end == '/')
> - m->mask = (uint32_t) strtoul(end+1, NULL, 0);
> - else
> + if (*endptr == '/') {
> + mask = (uint32_t) strtoul(endptr + 1, &endptr, 0);
^^^^^^^^^^
No need for this cast.
J.
> + if (endptr == arg ||
> + *endptr != '\0' ||
> + (val == ULONG_MAX && errno == ERANGE) ||
> + val > UINT32_MAX)
> + return -1;
> +
> + m->mask = mask;
> + } else {
> m->mask = ~0;
> + }
> +
> + return 0;
> }
>
> static int
> @@ -3115,7 +3133,9 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
> break;
> case 'm':
> options |= opt2type[c];
> - parse_u32_mask(optarg, &tmpl->mark);
> + if (parse_u32_mask(optarg, &tmpl->mark) < 0)
> + exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
> +
> tmpl->filter_mark_kernel.val = tmpl->mark.value;
> tmpl->filter_mark_kernel.mask = tmpl->mark.mask;
> tmpl->filter_mark_kernel_set = true;
> --
> 2.30.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH conntrack,v2 2/2] conntrack: improve --mark parser
2024-10-21 19:10 ` Jeremy Sowden
@ 2024-10-22 9:49 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-22 9:49 UTC (permalink / raw)
To: Jeremy Sowden; +Cc: netfilter-devel
On Mon, Oct 21, 2024 at 08:10:56PM +0100, Jeremy Sowden wrote:
> On 2024-10-13, at 00:00:30 +0200, Pablo Neira Ayuso wrote:
> > Enhance helper function to parse mark and mask (if available), bail out
> > if input is not correct.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > v2: - remove value == 0 && errno == ERANGE check
> >
> > src/conntrack.c | 34 +++++++++++++++++++++++++++-------
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/conntrack.c b/src/conntrack.c
> > index 18829dbf79bc..5bd966cad657 100644
> > --- a/src/conntrack.c
> > +++ b/src/conntrack.c
> > @@ -1233,17 +1233,35 @@ static int parse_value(const char *str, uint32_t *ret, uint64_t max)
> > return 0;
> > }
> >
> > -static void
> > +static int
> > parse_u32_mask(const char *arg, struct u32_mask *m)
> > {
> > - char *end;
> > + uint64_t val, mask;
> > + char *endptr;
> > +
> > + val = strtoul(arg, &endptr, 0);
> > + if (endptr == arg ||
> > + (*endptr != '\0' && *endptr != '/') ||
> > + (val == ULONG_MAX && errno == ERANGE) ||
> > + val > UINT32_MAX)
> > + return -1;
> >
> > - m->value = (uint32_t) strtoul(arg, &end, 0);
> > + m->value = val;
> >
> > - if (*end == '/')
> > - m->mask = (uint32_t) strtoul(end+1, NULL, 0);
> > - else
> > + if (*endptr == '/') {
> > + mask = (uint32_t) strtoul(endptr + 1, &endptr, 0);
> ^^^^^^^^^^
>
> No need for this cast.
Amended and pushed out, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser
2024-10-12 22:00 [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Pablo Neira Ayuso
2024-10-12 22:00 ` [PATCH conntrack,v2 2/2] conntrack: improve --mark parser Pablo Neira Ayuso
@ 2024-10-21 19:10 ` Jeremy Sowden
1 sibling, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2024-10-21 19:10 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]
On 2024-10-13, at 00:00:29 +0200, Pablo Neira Ayuso wrote:
> strtoul() is called with no error checking at all, add a helper
> function to validate input is correct for values less than
> UINT32_MAX.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> v2: - remove value == 0 && errno == ERANGE check
> - add assert to remember this only supports max up to UINT32_MAX
LGTM.
J.
> src/conntrack.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/src/conntrack.c b/src/conntrack.c
> index 9fa49869b553..18829dbf79bc 100644
> --- a/src/conntrack.c
> +++ b/src/conntrack.c
> @@ -1213,6 +1213,26 @@ parse_parameter_mask(const char *arg, unsigned int *status, unsigned int *mask,
> exit_error(PARAMETER_PROBLEM, "Bad parameter `%s'", arg);
> }
>
> +static int parse_value(const char *str, uint32_t *ret, uint64_t max)
> +{
> + char *endptr;
> + uint64_t val;
> +
> + assert(max <= UINT32_MAX);
> +
> + errno = 0;
> + val = strtoul(str, &endptr, 0);
> + if (endptr == str ||
> + *endptr != '\0' ||
> + (val == ULONG_MAX && errno == ERANGE) ||
> + val > max)
> + return -1;
> +
> + *ret = val;
> +
> + return 0;
> +}
> +
> static void
> parse_u32_mask(const char *arg, struct u32_mask *m)
> {
> @@ -2918,6 +2938,7 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
> struct ct_tmpl *tmpl;
> int res = 0, partial;
> union ct_address ad;
> + uint32_t value;
> int c, cmd;
>
> /* we release these objects in the exit_error() path. */
> @@ -3078,17 +3099,19 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
> case 'w':
> case '(':
> case ')':
> + if (parse_value(optarg, &value, UINT16_MAX) < 0)
> + exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
> +
> options |= opt2type[c];
> - nfct_set_attr_u16(tmpl->ct,
> - opt2attr[c],
> - strtoul(optarg, NULL, 0));
> + nfct_set_attr_u16(tmpl->ct, opt2attr[c], value);
> break;
> case 'i':
> case 'c':
> + if (parse_value(optarg, &value, UINT32_MAX) < 0)
> + exit_error(OTHER_PROBLEM, "unexpected value '%s' with -%c option", optarg, c);
> +
> options |= opt2type[c];
> - nfct_set_attr_u32(tmpl->ct,
> - opt2attr[c],
> - strtoul(optarg, NULL, 0));
> + nfct_set_attr_u32(tmpl->ct, opt2attr[c], value);
> break;
> case 'm':
> options |= opt2type[c];
> --
> 2.30.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-22 9:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-12 22:00 [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Pablo Neira Ayuso
2024-10-12 22:00 ` [PATCH conntrack,v2 2/2] conntrack: improve --mark parser Pablo Neira Ayuso
2024-10-21 19:10 ` Jeremy Sowden
2024-10-22 9:49 ` Pablo Neira Ayuso
2024-10-21 19:10 ` [PATCH conntrack,v2 1/2] conntrack: improve --secmark,--id,--zone parser Jeremy Sowden
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).