From 80e55ef8ba5b51eff6477d465b4326d94388ebf0 Mon Sep 17 00:00:00 2001
From: Kubat <mael.martin31@gmail.com>
Date: Sun, 23 May 2021 20:32:33 +0200
Subject: [PATCH] CMD: Limit the command name size to avoid big recursive calls
We do that because an ill-intentioned client might send a 2000 character
long command name to try to SEGV the lektord process because of a stack
to small to handle all the recursive calls.
The recursive calls are done in the trie find function, where the stack
consumed by each calls increases each time a recursive call is done
(indexes and other local variables...).
---
inc/lektor/common.h | 13 +++++----
src/net/command.c | 69 ++++++++++++++++++++++++++++++++-------------
2 files changed, 57 insertions(+), 25 deletions(-)
diff --git a/inc/lektor/common.h b/inc/lektor/common.h
index 82a58096..5e80b94b 100644
--- a/inc/lektor/common.h
+++ b/inc/lektor/common.h
@@ -136,14 +136,15 @@ void ___lkt_print(const char *section, const char *format, ...);
/* Default iteration count in search_iter */
#define LKT_DB_DEFAULT_ITERATION_MAX 100
-#define LEKTOR_TAG_MAX 256
-#define LKT_MESSAGE_ARGS_MAX 32
-#define LKT_MESSAGE_MAX 2048
-#define LKT_DEFAULT_LIST_SIZE 10
+#define LEKTOR_TAG_MAX 256 /* Max length of a tag */
+#define LKT_COMMAND_LEN_MAX 32 /* MPD commands are at most 32 character long */
+#define LKT_MESSAGE_ARGS_MAX 32 /* At most 32 words in a command are supported */
+#define LKT_MESSAGE_MAX 2048 /* A message is at most <defined> chars long */
+#define LKT_DEFAULT_LIST_SIZE 10
#define LKT_BACKLOG 32
-#define COMMAND_LIST_MAX 64
-#define BUFFER_OUT_MAX 16
+#define COMMAND_LIST_MAX 64 /* At most 64 commands per client */
+#define BUFFER_OUT_MAX 16 /* At most 16 messages per client */
#define MPD_VERSION "0.21.16"
typedef volatile enum {
diff --git a/src/net/command.c b/src/net/command.c
index 0f9321ce..3d72a8bd 100644
--- a/src/net/command.c
+++ b/src/net/command.c
@@ -3,39 +3,64 @@
#include <lektor/common.h>
#include <lektor/net.h>
+PRIVATE_FUNCTION void
+___crop_string(char *string, const size_t max_len)
+{
+ if (strlen(string) > max_len) {
+ string[max_len] = '\0';
+ }
+}
+
struct lkt_command
lkt_command_parse(char *raw)
{
errno = 0;
char *endptr;
- struct lkt_command res = { .name = raw, .args = { 0 }, .cont = strtol(raw, &endptr, 10) };
+ bool skip_continuation = false;
+ struct lkt_command res = {
+ .name = raw,
+ .args = { 0 },
+ .cont = strtol(raw, &endptr, 10)
+ };
+ /* Do we need the continuation? */
if (errno != 0) {
res.cont = 0;
- goto skip_cont;
- }
-
- if (endptr == raw) {
+ skip_continuation = true;
+ } else if (endptr == raw) {
res.cont = 0;
- goto skip_cont;
+ skip_continuation = true;
}
- if (*endptr == '\0') {
+ /* Need the continuation here. */
+ if (!skip_continuation) {
/* That thing is an error. */
- return res;
- }
+ if (*endptr == '\0') {
+ ___crop_string(res.name, LKT_COMMAND_LEN_MAX);
+ return res;
+ }
- /* Get the real command name. */
- raw += strspn(raw, " 1234567890");
- if (!*raw)
- return res;
- res.name = raw;
+ /* Get the real command name and not the continuation number. */
+ raw += strspn(raw, " 1234567890");
+ if (!*raw) {
+ /* No real name found, only got integers, we will fail later. */
+ ___crop_string(res.name, LKT_COMMAND_LEN_MAX);
+ return res;
+ }
+ res.name = raw;
+ }
- /* Get the args. */
-skip_cont:
+ /* Get the args. We skip the first word in the 'raw'. */
raw += strcspn(raw, " ");
- if (!*raw)
+
+ /* We also restrict the number of characters of the command, to avoid some
+ * incredibly hight recursive calls... */
+ ___crop_string(res.name, LKT_COMMAND_LEN_MAX);
+
+ if (!*raw) {
+ /* Here we have only one argument. */
return res;
+ }
*raw++ = 0;
raw += strspn(raw, " ");
@@ -49,7 +74,9 @@ skip_cont:
raw += strcspn(raw, " ");
*raw++ = 0;
}
- } else if (*raw == '\'') {
+ }
+
+ else if (*raw == '\'') {
res.args[i] = ++raw;
raw += strcspn(raw, "'");
if (*raw) {
@@ -57,14 +84,18 @@ skip_cont:
raw += strcspn(raw, " ");
*raw++ = 0;
}
- } else {
+ }
+
+ else {
res.args[i] = raw;
raw += strcspn(raw, " ");
*raw++ = 0;
}
+
raw += strspn(raw, " ");
}
+ /* The command has arguments. */
return res;
}
--
GitLab