Skip to content

Commit 36a72dd

Browse files
rluboscfriedt
authored andcommitted
net: lwm2m: Fix JSON write handling
There were several issues preventing JSON format writes to work correctly: 1. The formatter wrongly assumed that Base Name and Relative Name values read from the message are NULL terminated, which in result could give invalid results when combining them. 2. The formatter wrongly assumed that Relative Name is always present, which is not always the case. In result, it failed to parse messages, which contained full path in their Base Name Field 3. There were no boundaries check when reading JSON variable name/value, which could lead to buffer overflow in case malformed data was received. Signed-off-by: Robert Lubos <[email protected]>
1 parent a98d47b commit 36a72dd

File tree

1 file changed

+36
-11
lines changed

1 file changed

+36
-11
lines changed

subsys/net/lib/lwm2m/lwm2m_rw_json.c

+36-11
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,12 @@ int do_write_op_json(struct lwm2m_message *msg)
827827

828828
/* PARSE base name "bn" */
829829
json_next_token(&msg->in, &fd);
830+
831+
if (fd.value_len >= sizeof(base_name)) {
832+
LOG_ERR("Base name too long");
833+
return -EINVAL;
834+
}
835+
830836
/* TODO: validate name == "bn" */
831837
if (buf_read(base_name, fd.value_len,
832838
CPKT_BUF_READ(msg->in.in_cpkt),
@@ -835,39 +841,60 @@ int do_write_op_json(struct lwm2m_message *msg)
835841
return -EINVAL;
836842
}
837843

838-
/* skip to elements */
844+
base_name[fd.value_len] = '\0';
845+
846+
/* Relative name is optional - preinitialize full name with base name */
847+
snprintk(full_name, sizeof(full_name), "%s", base_name);
848+
849+
/* skip to elements ("e")*/
839850
json_next_token(&msg->in, &fd);
840-
/* TODO: validate name == "bv" */
841851

842852
while (json_next_token(&msg->in, &fd)) {
843853

844854
if (!(fd.json_flags & T_VALUE)) {
845855
continue;
846856
}
847857

858+
if (fd.name_len > sizeof(value)) {
859+
LOG_ERR("Token value too long");
860+
ret = -EINVAL;
861+
break;
862+
}
863+
848864
if (buf_read(value, fd.name_len,
849865
CPKT_BUF_READ(msg->in.in_cpkt),
850866
&fd.name_offset) < 0) {
851867
LOG_ERR("Error parsing name!");
852-
continue;
868+
ret = -EINVAL;
869+
break;
853870
}
854871

855-
/* handle resource name */
856872
if (value[0] == 'n') {
857-
/* reset values */
858-
created = 0U;
873+
/* handle resource name */
874+
if (fd.value_len >= sizeof(value)) {
875+
LOG_ERR("Relative name too long");
876+
ret = -EINVAL;
877+
break;
878+
}
859879

860880
/* get value for relative path */
861881
if (buf_read(value, fd.value_len,
862882
CPKT_BUF_READ(msg->in.in_cpkt),
863883
&fd.value_offset) < 0) {
864884
LOG_ERR("Error parsing relative path!");
865-
continue;
885+
ret = -EINVAL;
886+
break;
866887
}
867888

889+
value[fd.value_len] = '\0';
890+
868891
/* combine base_name + name */
869892
snprintk(full_name, sizeof(full_name), "%s%s",
870893
base_name, value);
894+
} else {
895+
/* handle resource value */
896+
/* reset values */
897+
created = 0U;
871898

872899
/* parse full_name into path */
873900
ret = parse_path(full_name, strlen(full_name),
@@ -939,16 +966,14 @@ int do_write_op_json(struct lwm2m_message *msg)
939966
ret = -ENOENT;
940967
break;
941968
}
942-
} else if (res && res_inst) {
943-
/* handle value assignment */
969+
970+
/* Write the resource value */
944971
ret = lwm2m_write_handler(obj_inst, res, res_inst,
945972
obj_field, msg);
946973
if (orig_path.level >= 3U && ret < 0) {
947974
/* return errors on a single write */
948975
break;
949976
}
950-
} else {
951-
/* complain about error? */
952977
}
953978
}
954979

0 commit comments

Comments
 (0)