From a88c7219793a99c11dffb4c5244390a4508997d1 Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Sun, 10 Nov 2024 20:40:51 +0000 Subject: [PATCH] environment: ignore env var assignments > 1 KiB (#2325) ...to guard against recursive constructs like FOO=$FOO:bar which would grow on each reconfigure. Add log message as well as a warning against this in the man page. --- docs/labwc-config.5.scd | 11 ++++++++--- src/config/session.c | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/labwc-config.5.scd b/docs/labwc-config.5.scd index 636172e7..66a8d611 100644 --- a/docs/labwc-config.5.scd +++ b/docs/labwc-config.5.scd @@ -50,9 +50,14 @@ that directory contains either the "environment" file or at least one Note: environment files are treated differently by Openbox, which will simply source the file as a valid shell script before running the window manager. Files -are instead parsed directly by labwc, although any environment variables -referenced as $VARIABLE or ${VARIABLE} will be substituted and the tilde (~) -will be expanded as the user's home directory. +are instead parsed directly by labwc so that environment variables can be +re-loaded on --reconfigure. + +Any environment variables referenced as $VARIABLE or ${VARIABLE} will be +substituted and the tilde (~) will be expanded as the user's home directory. + +Please note that as labwc reloads the environment file(s) on reconfigure, +recursive/circular assignments (for example FOO=$FOO:bar) should not be made. The *autostart* file is executed as a shell script after labwc has read its configuration and set variables defined in the environment file. Additionally, diff --git a/src/config/session.c b/src/config/session.c index c488c6b4..cfd0b4f4 100644 --- a/src/config/session.c +++ b/src/config/session.c @@ -33,6 +33,7 @@ static const char *const env_vars[] = { NULL }; +#define LAB_ENV_VAR_MAX_SIZE 1024 static void process_line(char *line) { @@ -52,12 +53,36 @@ process_line(char *line) struct buf value = BUF_INIT; buf_add(&value, string_strip(++p)); + + /* + * Users should not assign environment variables recursively (for + * example FOO=$FOO:bar) in the environment file because they would just + * keep growing on reconfigure. We could of course try to remember the + * original values, but it adds complexity for which we currently cannot + * see a use-case that could not easily be handled by some shell scripts + * separate from the environment file. + * + * Consequently we just check the size of the environment variables to + * defensively handle that growth issue in the case of inadvertent + * recursion. + */ + if (value.len > LAB_ENV_VAR_MAX_SIZE) { + wlr_log(WLR_ERROR, "ignoring environment variable assignment as " + "its size is greater than %d bytes which indicates recursion " + "(%s=%s)", LAB_ENV_VAR_MAX_SIZE, key, value.data); + goto err; + } + buf_expand_shell_variables(&value); buf_expand_tilde(&value); setenv(key, value.data, 1); + +err: buf_reset(&value); } +#undef LAB_ENV_VAR_MAX_SIZE + /* return true on successful read */ static bool read_environment_file(const char *filename)