* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.