| Message ID | 1585078044-14974-2-git-send-email-prathyusha.n@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Brian Gix |
| Headers | show |
| Series | [1/1] mesh: Check limits for count and interval of relay | expand |
Hi Prathyusha,
Inga and I have been talking, and comparing behavior to spec, and we believe that the *only* way that invalid
data can make its way into the system is to hand-edit the node.json file, which is not something we want to
encourage. We suggest moving validation to where the node.json is read....
mesh/config-json.c in parse_features()
Basically, we only care if
"mode":"enabled",
in which case, we range check interval (10-320) and count (1-8).
Any values outside that range, we "Fail to Parse" the node, and that node will not be loaded.... So edit the
node.json by hand *only* with legal arguments.
Beyond that, if relay is "disabled" or "unsupported", the interval and count are don't cares, and there is no
obligation for the 2nd parameter octet of RELAY_STATUS to be zero'd. As for the incoming RELAY_SET, there are
no out of range or disallowed values for count or interval.
On Wed, 2020-03-25 at 00:57 +0530, Prathyusha Nelluri wrote:
> From: Prathyusha N <prathyusha.n@samsung.com>
>
> Added limit checking condition for count and interval
> before processing for count and interval steps.
> ---
> mesh/cfgmod-server.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
> index 7111411c7..151cab154 100644
> --- a/mesh/cfgmod-server.c
> +++ b/mesh/cfgmod-server.c
> @@ -881,7 +881,11 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst, uint16_t unicast,
> n = mesh_model_opcode_set(OP_CONFIG_RELAY_STATUS, msg);
>
> msg[n++] = node_relay_mode_get(node, &count, &interval);
> - msg[n++] = (count - 1) + ((interval/10 - 1) << 3);
> +
> + if (count > 0 && interval >= 10)
> + msg[n++] = (count - 1) + ((interval/10 - 1) << 3);
> + else
> + msg[n++] = 0;
>
> l_debug("Get/Set Relay Config (%d)", msg[n-1]);
> break;
Applied On Wed, 2020-03-25 at 19:12 +0000, Gix, Brian wrote: > Hi Prathyusha, > > Inga and I have been talking, and comparing behavior to spec, and we believe that the *only* way that invalid > data can make its way into the system is to hand-edit the node.json file, which is not something we want to > encourage. We suggest moving validation to where the node.json is read.... > mesh/config-json.c in parse_features() > > Basically, we only care if > "mode":"enabled", > > in which case, we range check interval (10-320) and count (1-8). > > Any values outside that range, we "Fail to Parse" the node, and that node will not be loaded.... So edit the > node.json by hand *only* with legal arguments. > > Beyond that, if relay is "disabled" or "unsupported", the interval and count are don't cares, and there is no > obligation for the 2nd parameter octet of RELAY_STATUS to be zero'd. As for the incoming RELAY_SET, there > are > no out of range or disallowed values for count or interval. > > > On Wed, 2020-03-25 at 00:57 +0530, Prathyusha Nelluri wrote: > > From: Prathyusha N <prathyusha.n@samsung.com> > > > > Added limit checking condition for count and interval > > before processing for count and interval steps. > > --- > > mesh/cfgmod-server.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c > > index 7111411c7..151cab154 100644 > > --- a/mesh/cfgmod-server.c > > +++ b/mesh/cfgmod-server.c > > @@ -881,7 +881,11 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst, uint16_t unicast, > > n = mesh_model_opcode_set(OP_CONFIG_RELAY_STATUS, msg); > > > > msg[n++] = node_relay_mode_get(node, &count, &interval); > > - msg[n++] = (count - 1) + ((interval/10 - 1) << 3); > > + > > + if (count > 0 && interval >= 10) > > + msg[n++] = (count - 1) + ((interval/10 - 1) << 3); > > + else > > + msg[n++] = 0; > > > > l_debug("Get/Set Relay Config (%d)", msg[n-1]); > > break;
diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c index 7111411c7..151cab154 100644 --- a/mesh/cfgmod-server.c +++ b/mesh/cfgmod-server.c @@ -881,7 +881,11 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst, uint16_t unicast, n = mesh_model_opcode_set(OP_CONFIG_RELAY_STATUS, msg); msg[n++] = node_relay_mode_get(node, &count, &interval); - msg[n++] = (count - 1) + ((interval/10 - 1) << 3); + + if (count > 0 && interval >= 10) + msg[n++] = (count - 1) + ((interval/10 - 1) << 3); + else + msg[n++] = 0; l_debug("Get/Set Relay Config (%d)", msg[n-1]); break;
From: Prathyusha N <prathyusha.n@samsung.com> Added limit checking condition for count and interval before processing for count and interval steps. --- mesh/cfgmod-server.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)