summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorNeels Hofmeyr <neels@hofmeyr.de>2017-09-26 14:21:44 +0200
committerHarald Welte <laforge@gnumonks.org>2017-12-20 15:50:24 +0000
commit3da9aa6b6792407d57816ec4a4cdfd0b4b3434b8 (patch)
tree1bcb71940b90c2f3a1e27e0aab5a777c0f52489e /src
parent6cb9e7d8981e127161f14f22ad9271252c531aec (diff)
ctrl: tighten CTRL input parsing
Validate that incoming CTRL commands... - have decimal IDs, - return error on trailing characters, - have invalid characters in variable identifiers, - send detailed error messages as reply to the requestor. Adjust ctrl_test.{c,ok}, which best show the change in behavior. Message handling causes log messages on stderr; previously, stderr was empty. Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test failures. Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
Diffstat (limited to 'src')
-rw-r--r--src/ctrl/control_cmd.c77
1 files changed, 70 insertions, 7 deletions
diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c
index c2ce2be4..f32a200c 100644
--- a/src/ctrl/control_cmd.c
+++ b/src/ctrl/control_cmd.c
@@ -281,6 +281,15 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)
return res;
}
+static bool id_str_valid(const char *str)
+{
+ for (;*str;str++) {
+ if (!isdigit(*str))
+ return false;
+ }
+ return true;
+}
+
/*! Parse CTRL command struct from msgb, return ctrl->type == CTRL_TYPE_ERROR and an error message in
* ctrl->reply on any error.
* The caller is responsible to talloc_free() the returned struct pointer. */
@@ -306,6 +315,7 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
cmd->type = CTRL_TYPE_ERROR;
cmd->id = "err";
cmd->reply = "Request malformed";
+ LOGP(DLCTRL, LOGL_NOTICE, "Malformed request: \"%s\"\n", osmo_escape_str(str, -1));
goto err;
}
@@ -314,6 +324,7 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
cmd->type = CTRL_TYPE_ERROR;
cmd->id = "err";
cmd->reply = "Request type unknown";
+ LOGP(DLCTRL, LOGL_NOTICE, "Request type unknown: \"%s\"\n", osmo_escape_str(str, -1));
goto err;
}
@@ -323,6 +334,15 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
cmd->type = CTRL_TYPE_ERROR;
cmd->id = "err";
cmd->reply = "Missing ID";
+ LOGP(DLCTRL, LOGL_NOTICE, "Missing ID: \"%s\"\n", osmo_escape_str(str, -1));
+ goto err;
+ }
+
+ if (!id_str_valid(tmp)) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->id = "err";
+ cmd->reply = "Invalid message ID number";
+ LOGP(DLCTRL, LOGL_NOTICE, "Invalid message ID number: \"%s\"\n", osmo_escape_str(tmp, -1));
goto err;
}
cmd->id = talloc_strdup(cmd, tmp);
@@ -331,14 +351,30 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
switch (cmd->type) {
case CTRL_TYPE_GET:
- var = strtok_r(NULL, " ", &saveptr);
+ var = strtok_r(NULL, " \n", &saveptr);
if (!var) {
cmd->type = CTRL_TYPE_ERROR;
cmd->reply = "GET incomplete";
- LOGP(DLCTRL, LOGL_NOTICE, "GET Command incomplete\n");
+ LOGP(DLCTRL, LOGL_NOTICE, "GET Command incomplete: \"%s\"\n",
+ osmo_escape_str(str, -1));
+ goto err;
+ }
+ if (!osmo_separated_identifiers_valid(var, ".")) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->reply = "GET variable contains invalid characters";
+ LOGP(DLCTRL, LOGL_NOTICE, "GET variable contains invalid characters: \"%s\"\n",
+ osmo_escape_str(var, -1));
goto err;
}
cmd->variable = talloc_strdup(cmd, var);
+ var = strtok_r(NULL, "", &saveptr);
+ if (var) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->reply = "GET with trailing characters";
+ LOGP(DLCTRL, LOGL_NOTICE, "GET with trailing characters: \"%s\"\n",
+ osmo_escape_str(var, -1));
+ goto err;
+ }
LOGP(DLCTRL, LOGL_DEBUG, "Command: GET %s\n", cmd->variable);
break;
case CTRL_TYPE_SET:
@@ -350,31 +386,57 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
LOGP(DLCTRL, LOGL_NOTICE, "SET Command incomplete\n");
goto err;
}
+ if (!osmo_separated_identifiers_valid(var, ".")) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->reply = "SET variable contains invalid characters";
+ LOGP(DLCTRL, LOGL_NOTICE, "SET variable contains invalid characters: \"%s\"\n",
+ osmo_escape_str(var, -1));
+ goto err;
+ }
cmd->variable = talloc_strdup(cmd, var);
cmd->value = talloc_strdup(cmd, val);
if (!cmd->variable || !cmd->value)
goto oom;
- LOGP(DLCTRL, LOGL_DEBUG, "Command: SET %s = %s\n", cmd->variable, cmd->value);
+
+ var = strtok_r(NULL, "", &saveptr);
+ if (var) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->reply = "SET with trailing characters";
+ LOGP(DLCTRL, LOGL_NOTICE, "SET with trailing characters: \"%s\"\n",
+ osmo_escape_str(var, -1));
+ goto err;
+ }
+
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: SET %s = \"%s\"\n", cmd->variable,
+ osmo_escape_str(cmd->value, -1));
break;
case CTRL_TYPE_GET_REPLY:
case CTRL_TYPE_SET_REPLY:
case CTRL_TYPE_TRAP:
var = strtok_r(NULL, " ", &saveptr);
- val = strtok_r(NULL, " ", &saveptr);
+ val = strtok_r(NULL, "", &saveptr);
if (!var || !val) {
cmd->type = CTRL_TYPE_ERROR;
cmd->reply = "Trap/Reply incomplete";
LOGP(DLCTRL, LOGL_NOTICE, "Trap/Reply incomplete\n");
goto err;
}
+ if (!osmo_separated_identifiers_valid(var, ".")) {
+ cmd->type = CTRL_TYPE_ERROR;
+ cmd->reply = "Trap/Reply variable contains invalid characters";
+ LOGP(DLCTRL, LOGL_NOTICE, "Trap/Reply variable contains invalid characters: \"%s\"\n",
+ osmo_escape_str(var, -1));
+ goto err;
+ }
cmd->variable = talloc_strdup(cmd, var);
cmd->reply = talloc_strdup(cmd, val);
if (!cmd->variable || !cmd->reply)
goto oom;
- LOGP(DLCTRL, LOGL_DEBUG, "Command: TRAP/REPLY %s: %s\n", cmd->variable, cmd->reply);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: TRAP/REPLY %s: \"%s\"\n", cmd->variable,
+ osmo_escape_str(cmd->reply, -1));
break;
case CTRL_TYPE_ERROR:
- var = strtok_r(NULL, "\0", &saveptr);
+ var = strtok_r(NULL, "", &saveptr);
if (!var) {
cmd->reply = "";
goto err;
@@ -382,7 +444,8 @@ struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg)
cmd->reply = talloc_strdup(cmd, var);
if (!cmd->reply)
goto oom;
- LOGP(DLCTRL, LOGL_DEBUG, "Command: ERROR %s\n", cmd->reply);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: ERROR \"%s\"\n",
+ osmo_escape_str(cmd->reply, -1));
break;
case CTRL_TYPE_UNKNOWN:
default: