From f2814458dd39318b981413bb2dfb6b8591606d88 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 17 Aug 2026 09:24:52 +0200 Subject: [PATCH] omfile: harden dynafile default containment Why: dynaFile path flexibility is needed by existing deployments, but paths derived from message data need a safer default. Impact: dynaFile output is lexically contained below its configured base by default, while an explicit warned compatibility opt-in remains available. Before/After: Rendered paths could escape the intended base; they are now blocked by default unless the legacy override is explicitly enabled. Technical Overview: Derive a normalized fixed base from string and list templates. Check rendered dynafile paths against that base before they are opened. Use fallback guards for legacy templates without an inspectable base. Expose dangerousPermitPathEscape as a deliberate warned opt-in. Keep legacy template forms usable with compatibility controls. Document securePath and secpath-replace as the preferred protection. Add RainerScript and YAML regression coverage. With the help of AI-Agents: Codex --- doc/Makefile.am | 2 + doc/source/configuration/actions.rst | 10 +- doc/source/configuration/modules/omfile.rst | 14 + .../configuration/property_replacer.rst | 12 +- doc/source/configuration/templates.rst | 14 + doc/source/configuration/yaml_config.rst | 8 +- ...ile-dynafile-dangerouspermitpathescape.rst | 73 ++++ .../omfile-dynafile-restricttemplatetype.rst | 61 ++++ .../reference/parameters/omfile-dynafile.rst | 45 ++- .../templates/templates-examples.rst | 10 +- .../templates-statement-property.rst | 25 +- .../templates/templates-type-string.rst | 12 + tests/Makefile.am | 8 + tests/dynfile_invld_sync.sh | 6 +- .../omfile-dynafile-template-type-invalid.sh | 33 ++ ...ile-dynafile-template-type-list-default.sh | 29 ++ .../omfile-dynafile-template-type-override.sh | 30 ++ ...ile-dynafile-traversal-default-template.sh | 48 +++ ...omfile-dynafile-traversal-list-template.sh | 43 +++ ...omfile-dynafile-traversal-permit-escape.sh | 54 +++ tests/omfile-dynafile-traversal-repro.sh | 37 ++ ...omfile-dynafile-traversal-list-template.sh | 91 +++++ tools/omfile.c | 334 ++++++++++++++++++ 23 files changed, 985 insertions(+), 14 deletions(-) create mode 100644 doc/source/reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst create mode 100644 doc/source/reference/parameters/omfile-dynafile-restricttemplatetype.rst create mode 100755 tests/omfile-dynafile-template-type-invalid.sh create mode 100755 tests/omfile-dynafile-template-type-list-default.sh create mode 100755 tests/omfile-dynafile-template-type-override.sh create mode 100755 tests/omfile-dynafile-traversal-default-template.sh create mode 100755 tests/omfile-dynafile-traversal-list-template.sh create mode 100755 tests/omfile-dynafile-traversal-permit-escape.sh create mode 100755 tests/omfile-dynafile-traversal-repro.sh create mode 100755 tests/yaml-omfile-dynafile-traversal-list-template.sh diff --git a/doc/Makefile.am b/doc/Makefile.am index 6ce3ce0c3..2fb70749e 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1044,7 +1044,9 @@ EXTRA_DIST = \ source/reference/parameters/omfile-dirgroupnum.rst \ source/reference/parameters/omfile-dirowner.rst \ source/reference/parameters/omfile-dirownernum.rst \ + source/reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst \ source/reference/parameters/omfile-dynafile-donotsuspend.rst \ + source/reference/parameters/omfile-dynafile-restricttemplatetype.rst \ source/reference/parameters/omfile-dynafile.rst \ source/reference/parameters/omfile-dynafilecachesize.rst \ source/reference/parameters/omfile-failonchownfailure.rst \ diff --git a/doc/source/configuration/actions.rst b/doc/source/configuration/actions.rst index 5ebc7020a..b5667ef0a 100644 --- a/doc/source/configuration/actions.rst +++ b/doc/source/configuration/actions.rst @@ -332,7 +332,15 @@ file for it. **Creating directories is also supported**. For example you can use the hostname as directory and the program name as file name: - ``$template DynFile,"/var/log/%HOSTNAME%/%programname%.log"`` + ``$template DynFile,"/var/log/%HOSTNAME:::secpath-replace%/%programname:::secpath-replace%.log"`` + +When a dynafile template uses message properties in path components, +escape each message-derived component. The ``secpath-replace`` property +option replaces path separators with ``_`` and makes special ``.`` and +``..`` components non-special, which prevents remote messages from +selecting parent directories or absolute paths. Apply it to every field +that can come from the message or sender, such as ``HOSTNAME``, +``programname``, ``APP-NAME``, or data extracted into variables. Named Pipes ~~~~~~~~~~~ diff --git a/doc/source/configuration/modules/omfile.rst b/doc/source/configuration/modules/omfile.rst index eff6e5841..3fbc191af 100644 --- a/doc/source/configuration/modules/omfile.rst +++ b/doc/source/configuration/modules/omfile.rst @@ -74,7 +74,9 @@ about different configuration languages in use by rsyslog. ../../reference/parameters/omfile-dirowner ../../reference/parameters/omfile-dirownernum ../../reference/parameters/omfile-dynafile + ../../reference/parameters/omfile-dynafile-dangerouspermitpathescape ../../reference/parameters/omfile-dynafile-donotsuspend + ../../reference/parameters/omfile-dynafile-restricttemplatetype ../../reference/parameters/omfile-dynafilecachesize ../../reference/parameters/omfile-failonchownfailure ../../reference/parameters/omfile-file @@ -167,6 +169,14 @@ Module Parameters - .. include:: ../../reference/parameters/omfile-dynafile-donotsuspend.rst :start-after: .. summary-start :end-before: .. summary-end + * - :ref:`param-omfile-dynafile-dangerouspermitpathescape` + - .. include:: ../../reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst + :start-after: .. summary-start + :end-before: .. summary-end + * - :ref:`param-omfile-dynafile-restricttemplatetype` + - .. include:: ../../reference/parameters/omfile-dynafile-restricttemplatetype.rst + :start-after: .. summary-start + :end-before: .. summary-end * - :ref:`param-omfile-compression-driver` - .. include:: ../../reference/parameters/omfile-compression-driver.rst :start-after: .. summary-start @@ -238,6 +248,10 @@ selects whether a static or dynamic file (name) shall be written to. - .. include:: ../../reference/parameters/omfile-dynafile.rst :start-after: .. summary-start :end-before: .. summary-end + * - :ref:`param-omfile-dynafile-dangerouspermitpathescape` + - .. include:: ../../reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst + :start-after: .. summary-start + :end-before: .. summary-end * - :ref:`param-omfile-closetimeout` - .. include:: ../../reference/parameters/omfile-closetimeout.rst :start-after: .. summary-start diff --git a/doc/source/configuration/property_replacer.rst b/doc/source/configuration/property_replacer.rst index 4da565dac..d5664557e 100644 --- a/doc/source/configuration/property_replacer.rst +++ b/doc/source/configuration/property_replacer.rst @@ -346,12 +346,16 @@ options are defined: forget about it ;) **secpath-drop** - Drops slashes inside the field (e.g. "a/b" becomes "ab"). Useful for - secure pathname generation (with dynafiles). + Drops slashes inside the field (e.g. "a/b" becomes "ab") and makes + empty, "." and ".." path components non-special. Useful for secure + pathname generation with dynafiles, but may create name collisions + because separators are removed. **secpath-replace** - Replace slashes inside the field by an underscore. (e.g. "a/b" becomes - "a\_b"). Useful for secure pathname generation (with dynafiles). + Replaces slashes inside the field by an underscore (e.g. "a/b" becomes + "a\_b") and makes empty, "." and ".." path components non-special. + This is usually preferred for dynafile path components because it keeps + a visible separator while preventing path traversal. To use multiple options, simply place them one after each other with a comma delimiting them. For example "escape-cc,sp-if-no-1st-sp". If you diff --git a/doc/source/configuration/templates.rst b/doc/source/configuration/templates.rst index ebd95dcb8..c25ed3bed 100644 --- a/doc/source/configuration/templates.rst +++ b/doc/source/configuration/templates.rst @@ -130,6 +130,20 @@ If your output is a **structured JSON pipeline** (e.g. to Elasticsearch or a file), you do not need to add any textual header. +Templates for dynamic file names +-------------------------------- + +Templates used as ``omfile`` ``dynaFile`` names need additional care: +each message-derived path component must be escaped. Use +``secpath-replace`` in string templates, or ``securePath="replace"`` in +list template ``property()`` statements. Apply this to fields such as +``HOSTNAME``, ``programname``, ``APP-NAME``, and variables parsed from +the message. Fixed directory separators should remain in constant text. + +This prevents sender-controlled values from selecting parent directories +or absolute paths while keeping readable file names. + + .. _templates.template-object: The ``template()`` object diff --git a/doc/source/configuration/yaml_config.rst b/doc/source/configuration/yaml_config.rst index 8ee370ef8..3a2785883 100644 --- a/doc/source/configuration/yaml_config.rst +++ b/doc/source/configuration/yaml_config.rst @@ -209,7 +209,13 @@ placeholders: string: "%HOSTNAME% %syslogfacility-text%.%syslogseverity-text% %msg%\n" - name: filePerHost # use with omfile dynafile: filePerHost type: string - string: "/var/log/hosts/%HOSTNAME%.log" + string: "/var/log/hosts/%HOSTNAME:::secpath-replace%.log" + +When a template is used as an ``omfile`` dynafile name, escape every +message-derived path component. In string templates, use +``secpath-replace`` on fields such as ``HOSTNAME``, ``programname``, or +``APP-NAME`` so sender-controlled values cannot select parent +directories or absolute paths. *Subtree template* — serialises a JSON sub-tree of the message object: diff --git a/doc/source/reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst b/doc/source/reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst new file mode 100644 index 000000000..da6f1cf54 --- /dev/null +++ b/doc/source/reference/parameters/omfile-dynafile-dangerouspermitpathescape.rst @@ -0,0 +1,73 @@ +.. _param-omfile-dynafile-dangerouspermitpathescape: +.. _omfile.parameter.action.dynafile-dangerouspermitpathescape: + +dynafile.dangerousPermitPathEscape +================================== + +.. meta:: + :description: Permit omfile dynafile paths to escape the configured base path. + :keywords: rsyslog, omfile, dynafile, path traversal, security, dangerous + +.. index:: + single: omfile; dynafile.dangerousPermitPathEscape + single: dynafile.dangerousPermitPathEscape + +.. summary-start + +Dangerous fallback option that permits rendered dynafile paths to escape +the fixed path prefix configured in the dynafile template. + +.. summary-end + +This parameter applies to :doc:`../../configuration/modules/omfile`. + +:Name: dynafile.dangerousPermitPathEscape +:Scope: action +:Type: boolean +:Default: off +:Required?: no +:Introduced: not specified + +Description +----------- + +By default, omfile validates rendered :ref:`param-omfile-dynafile` paths +against the fixed path prefix configured in the dynafile template. For +example, a dynafile template that starts with ``/var/log/`` must render +paths below that configured base. Message-derived fields such as +``%HOSTNAME%`` must not use ``..`` components to escape that location. + +Set this parameter to ``on`` only as a temporary compatibility fallback +for the one affected action in a trusted legacy configuration. Enabling it allows +message-derived dynafile names to escape the configured path prefix. If +untrusted data can reach the dynafile template, rsyslog may create or +overwrite any file that the rsyslog process user is allowed to write. +Only operating-system permissions, mandatory access controls, mount +options, and similar external controls remain as protection. + +This also restores the historical behavior for opaque legacy dynafile +templates that render absolute paths or relative paths containing leading +``..`` components. Without this explicit opt-in, those paths are rejected by +the default fallback guard. + +Before enabling this option, reconsider the use case. Prefer changing the +dynafile template so that untrusted fields cannot select parent +directories or absolute paths. For network-sourced fields, keep this +option disabled. + +Action usage +------------ + +.. _param-omfile-action-dynafile-dangerouspermitpathescape: +.. code-block:: rsyslog + + action(type="omfile" + dynafile="legacyDynFile" + dynafile.dangerousPermitPathEscape="on") + +See also +-------- + +See also :doc:`../../configuration/modules/omfile`, +:doc:`omfile-dynafile`, and +:doc:`omfile-dynafile-restricttemplatetype`. diff --git a/doc/source/reference/parameters/omfile-dynafile-restricttemplatetype.rst b/doc/source/reference/parameters/omfile-dynafile-restricttemplatetype.rst new file mode 100644 index 000000000..6ab087f47 --- /dev/null +++ b/doc/source/reference/parameters/omfile-dynafile-restricttemplatetype.rst @@ -0,0 +1,61 @@ +.. _param-omfile-dynafile-restricttemplatetype: +.. _omfile.parameter.module.dynafile-restricttemplatetype: + +dynafile.restrictTemplateType +============================= + +.. meta:: + :description: Optionally restrict omfile dynafile templates to string and list types. + :keywords: rsyslog, omfile, dynafile, template, path traversal, security + +.. index:: + single: omfile; dynafile.restrictTemplateType + single: dynafile.restrictTemplateType + +.. summary-start + +Optionally restricts dynafile templates to string and list templates so omfile +can inspect their fixed path prefix before opening rendered paths. + +.. summary-end + +This parameter applies to :doc:`../../configuration/modules/omfile`. + +:Name: dynafile.restrictTemplateType +:Scope: module +:Type: boolean +:Default: module=off +:Required?: no +:Introduced: not specified + +Description +----------- + +When enabled, omfile accepts only ``type="string"`` and ``type="list"`` +templates for the :ref:`param-omfile-dynafile` parameter. These template +types expose their constant and property entries to omfile, which lets omfile +derive the trusted static directory prefix and block rendered paths that escape +it. + +The default preserves plugin/string-generator and subtree dynafile templates. +Those legacy forms use the fallback runtime guard: absolute paths and relative +paths that lexically escape through ``..`` are rejected. Use +``dynafile.dangerousPermitPathEscape="on"`` only when an affected legacy +configuration must retain that behavior. Enable this option when an +installation can use only string or list templates and wants that stricter +configuration-time policy. + +Module usage +------------ + +.. _param-omfile-module-dynafile-restricttemplatetype: +.. _omfile.parameter.module.dynafile-restricttemplatetype-usage: +.. code-block:: rsyslog + + module(load="builtin:omfile" dynafile.restrictTemplateType="on") + +See also +-------- + +See also :doc:`../../configuration/modules/omfile` and +:doc:`omfile-dynafile`. diff --git a/doc/source/reference/parameters/omfile-dynafile.rst b/doc/source/reference/parameters/omfile-dynafile.rst index ebeb4c855..d29bcac50 100644 --- a/doc/source/reference/parameters/omfile-dynafile.rst +++ b/doc/source/reference/parameters/omfile-dynafile.rst @@ -35,9 +35,48 @@ is just a regular :doc:`rsyslog template <../../configuration/templates>`, so you have full control over how to format the file name. To avoid path traversal attacks, *you must make sure that the template -used properly escapes file paths*. This is done by using the *securepath* -parameter in the template's property statements, or the *secpath-drop* -or *secpath-replace* property options with the property replacer. +properly escapes every message-derived path component*. In string +templates, use the *secpath-replace* property replacer option. In list +templates, use ``securePath="replace"`` on the corresponding +``property()`` statements. Apply this to all fields that can come from +the message or sender, such as ``HOSTNAME``, ``programname``, +``APP-NAME``, or parsed variables. Keep fixed directory separators in +constant text. + +By default, omfile also performs lexical containment checks for dynafile +paths. The rendered path must remain below the fixed leading directory +prefix that was configured in the dynafile template. This blocks remote +path traversal through components such as ``..`` or unexpected leading +slashes in message-derived fields, but it is not a filesystem sandbox. It +does not protect against symlinks, bind mounts, time-of-check/time-of-use +races, or malicious local filesystem state. For untrusted inputs, +``secpath-replace`` or ``securePath="replace"`` remains the strongest +recommended pattern. + +Backward-compatibility change +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Opaque legacy templates (subtree and plugin/string-generator templates) do +not provide a fixed prefix that omfile can inspect. They remain supported, but +their fallback guard rejects output paths that are absolute or lexically escape +through ``..``. This deliberately changes the historical behavior of affected +legacy configurations. To preserve such a configuration temporarily, set +:ref:`param-omfile-dynafile-dangerouspermitpathescape` to ``on`` on the +affected action (or module). That opt-in disables the guard for that action; +use it only with trusted path data. + +.. code-block:: rsyslog + + template(name="DynFile" type="string" + string="/var/log/hosts/%HOSTNAME:::secpath-replace%/%programname:::secpath-replace%.log") + + template(name="DynFileList" type="list") { + constant(value="/var/log/hosts/") + property(name="hostname" securePath="replace") + constant(value="/") + property(name="programname" securePath="replace") + constant(value=".log") + } When ``global(compatibility.defaults.secure="strict")`` is active, dynafile templates default fields without explicit secure path handling diff --git a/doc/source/reference/templates/templates-examples.rst b/doc/source/reference/templates/templates-examples.rst index cf60504fb..9910b3921 100644 --- a/doc/source/reference/templates/templates-examples.rst +++ b/doc/source/reference/templates/templates-examples.rst @@ -143,5 +143,11 @@ messages by host name: .. code-block:: none - template(name="DynFile" type="string" string="/var/log/system-%HOSTNAME%.log") - + template(name="DynFile" type="string" + string="/var/log/system-%HOSTNAME:::secpath-replace%.log") + +If a template is used with ``omfile`` ``dynaFile``, treat every +message-derived path component as untrusted. Use ``secpath-replace`` in +string templates, or ``securePath="replace"`` in list template property +statements, for fields such as ``HOSTNAME``, ``programname``, +``APP-NAME``, and variables extracted from the message. diff --git a/doc/source/reference/templates/templates-statement-property.rst b/doc/source/reference/templates/templates-statement-property.rst index e7e69c3f9..afaf636da 100644 --- a/doc/source/reference/templates/templates-statement-property.rst +++ b/doc/source/reference/templates/templates-statement-property.rst @@ -63,8 +63,11 @@ Parameters controlCharacters: escape-octal - constant: value: "\n" -- ``securePath`` – create safe paths for dynafile templates; ``drop`` or - ``replace`` +- ``securePath`` – escape a property for use as one path component in an + ``omfile`` dynafile template; ``drop`` removes path separators and + ``replace`` changes them to ``_``. Use ``replace`` for message-derived + fields such as host names, application names, program names, or parsed + variables. - ``format`` – field format. Supported values: - ``csv`` – generate CSV data @@ -121,3 +124,21 @@ Parameters - ``onEmpty`` – for ``jsonf`` format only; handling of empty values: ``keep``, ``skip``, or ``null`` + +Dynafile path components +------------------------ + +When a list template is used as an ``omfile`` ``dynaFile`` name, apply +``securePath="replace"`` to every property that can be influenced by the +message or sender. Fixed directory separators belong in ``constant()`` +statements; do not apply ``securePath`` to the complete path. + +.. code-block:: none + + template(name="DynFile" type="list") { + constant(value="/var/log/hosts/") + property(name="hostname" securePath="replace") + constant(value="/") + property(name="programname" securePath="replace") + constant(value=".log") + } diff --git a/doc/source/reference/templates/templates-type-string.rst b/doc/source/reference/templates/templates-type-string.rst index 8b4b071ba..30321fa62 100644 --- a/doc/source/reference/templates/templates-type-string.rst +++ b/doc/source/reference/templates/templates-type-string.rst @@ -26,3 +26,15 @@ The text between percent signs is interpreted by the property replacer, which reads message properties and applies options for formatting and processing. +When a string template is used as an ``omfile`` ``dynaFile`` name, +escape each message-derived path component with ``secpath-replace``. +This is needed for fields such as ``HOSTNAME``, ``programname``, +``APP-NAME``, or variables populated from message content: + +.. code-block:: none + + template(name="DynFile" type="string" + string="/var/log/hosts/%HOSTNAME:::secpath-replace%/%programname:::secpath-replace%.log") + +The escaping must be applied to the properties that form path +components, not to the whole path including fixed separators. diff --git a/tests/Makefile.am b/tests/Makefile.am index b9c14a674..cd6417109 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -279,6 +279,13 @@ TESTS_DEFAULT = \ omusrmsg-noabort.sh \ omusrmsg_ratelimit_name.sh \ omfile-module-params.sh \ + omfile-dynafile-traversal-default-template.sh \ + omfile-dynafile-traversal-list-template.sh \ + omfile-dynafile-traversal-permit-escape.sh \ + omfile-dynafile-traversal-repro.sh \ + omfile-dynafile-template-type-invalid.sh \ + omfile-dynafile-template-type-list-default.sh \ + omfile-dynafile-template-type-override.sh \ omfile-dynafilecachesize-invalid.sh \ omfile-dynafile-mmnormalize-property.sh \ omfile-read-only-errmsg.sh \ @@ -772,6 +779,7 @@ TESTS_LIBYAML = \ ratelimit_hup.sh \ yaml-basic.sh \ yaml-basic-yamlonly.sh \ + yaml-omfile-dynafile-traversal-list-template.sh \ yaml-omfile-dynafilecachesize-invalid.sh \ yaml-include.sh \ yaml-ruleset-script.sh \ diff --git a/tests/dynfile_invld_sync.sh b/tests/dynfile_invld_sync.sh index 0b8b1a428..16a9fb57d 100755 --- a/tests/dynfile_invld_sync.sh +++ b/tests/dynfile_invld_sync.sh @@ -14,7 +14,11 @@ $template dynfile,"%msg:F,58:2%.log" # complete name is in message $OMFileFlushOnTXEnd on $OMFileAsyncWriting off $DynaFileCacheSize 4 -local0.* ?dynfile;outfmt +# This test deliberately uses /proc to make the open fail after dynafile cache +# handling. The action-level compatibility option keeps that historic test +# stimulus available while containment remains the default for other actions. +local0.* action(type="omfile" dynafile="dynfile" template="outfmt" + dynafile.dangerousPermitPathEscape="on") ' startup # Send the handcrafted messages in one imdiag session so the invalid open diff --git a/tests/omfile-dynafile-template-type-invalid.sh b/tests/omfile-dynafile-template-type-invalid.sh new file mode 100755 index 000000000..7e67e019b --- /dev/null +++ b/tests/omfile-dynafile-template-type-invalid.sh @@ -0,0 +1,33 @@ +#!/bin/bash +# Ensure strict dynafile template-type mode rejects templates that cannot be inspected. +. ${srcdir:=.}/diag.sh init + +generate_conf +add_conf ' +module(load="builtin:omfile" dynafile.restrictTemplateType="on") + +template(name="dynfile" type="subtree" subtree="$!dynfile") +template(name="outfmt" type="string" string="%msg:F,58:2%\n") + +if $msg contains "msgnum:" then { + set $!dynfile = "'$RSYSLOG_OUT_LOG'"; + action(type="omfile" dynafile="dynfile" template="outfmt") +} +' + +../tools/rsyslogd -C -N1 -M"$RSYSLOG_MODDIR" -f"${TESTCONF_NM}.conf" \ + >"${RSYSLOG_DYNNAME}.log" 2>&1 +if [ $? -ne 1 ]; then + echo "FAIL: expected config validation failure for subtree dynafile template" + cat "${RSYSLOG_DYNNAME}.log" + error_exit 1 +fi + +grep -F "dynafile template 'dynfile' uses a template type that cannot be safely inspected" \ + "${RSYSLOG_DYNNAME}.log" >/dev/null || { + echo "FAIL: expected dynafile template type validation error" + cat "${RSYSLOG_DYNNAME}.log" + error_exit 1 +} + +exit_test diff --git a/tests/omfile-dynafile-template-type-list-default.sh b/tests/omfile-dynafile-template-type-list-default.sh new file mode 100755 index 000000000..a904ba0ad --- /dev/null +++ b/tests/omfile-dynafile-template-type-list-default.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Ensure normal list dynafile templates are accepted by default. +. ${srcdir:=.}/diag.sh init + +generate_conf +add_conf ' +template(name="dynfile" type="list") { + constant(value="./") + property(name="hostname") + constant(value="/") + property(name="app-name") + constant(value=".log") +} +template(name="outfmt" type="string" string="%msg:F,58:2%\n") + +if $msg contains "msgnum:" then { + action(type="omfile" dynafile="dynfile" template="outfmt") +} +' + +../tools/rsyslogd -C -N1 -M"$RSYSLOG_MODDIR" -f"${TESTCONF_NM}.conf" \ + >"${RSYSLOG_DYNNAME}.log" 2>&1 +if [ $? -ne 0 ]; then + echo "FAIL: expected list dynafile template to be accepted by default" + cat "${RSYSLOG_DYNNAME}.log" + error_exit 1 +fi + +exit_test diff --git a/tests/omfile-dynafile-template-type-override.sh b/tests/omfile-dynafile-template-type-override.sh new file mode 100755 index 000000000..c37762156 --- /dev/null +++ b/tests/omfile-dynafile-template-type-override.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Ensure legacy dynafile template types remain compatible under the fallback path guard. +. ${srcdir:=.}/diag.sh init +export NUMMESSAGES=1 + +generate_conf +add_conf ' +template(name="dynfile" type="subtree" subtree="$!dynfile") +template(name="outfmt" type="string" string="%msg:F,58:2%\n") + +if $msg contains "msgnum:" then { + set $!dynfile = "'$RSYSLOG_OUT_LOG'"; + action(type="omfile" dynafile="dynfile" template="outfmt") +} +' + +../tools/rsyslogd -C -N1 -M"$RSYSLOG_MODDIR" -f"${TESTCONF_NM}.conf" \ + >"${RSYSLOG_DYNNAME}.log" 2>&1 +if [ $? -ne 0 ]; then + echo "FAIL: dynafile template type override warning rejected the configuration" + cat "${RSYSLOG_DYNNAME}.log" + error_exit 1 +fi + +startup +injectmsg 0 "$NUMMESSAGES" +shutdown_when_empty +wait_shutdown +seq_check +exit_test diff --git a/tests/omfile-dynafile-traversal-default-template.sh b/tests/omfile-dynafile-traversal-default-template.sh new file mode 100755 index 000000000..22bdd127d --- /dev/null +++ b/tests/omfile-dynafile-traversal-default-template.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# Check dynafile traversal when omfile uses its default output template. +. ${srcdir:=.}/diag.sh init +require_plugin imtcp + +export TRAVERSAL_APP="${RSYSLOG_DYNNAME}-omfile-default-template-traversal" +export VALID_APP="${RSYSLOG_DYNNAME}-omfile-default-template-valid" +export TRAVERSAL_ROOT="${RSYSLOG_DYNNAME}.jail" +export TRAVERSAL_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.log" +export VALID_OUT="${TRAVERSAL_ROOT}/base/goodhost/${VALID_APP}.log" +rm -rf "$TRAVERSAL_ROOT" +mkdir -p "${TRAVERSAL_ROOT}/base" "${TRAVERSAL_ROOT}/escape" + +generate_conf +add_conf ' +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") +template(name="dynfile" type="string" string="'$TRAVERSAL_ROOT'/base/%HOSTNAME%/%APP-NAME%.log") + +$rulesetparser rsyslog.rfc5424 +local4.debug action(type="omfile" dynafile="dynfile") +' + +startup +{ + printf '<167>1 2003-03-01T01:00:00.000Z ../escape %s - - - traversal-default-template\n' \ + "$TRAVERSAL_APP" + printf '<167>1 2003-03-01T01:00:00.000Z goodhost %s - - - valid-after-blocked-traversal\n' \ + "$VALID_APP" +} > "${RSYSLOG_DYNNAME}.input" +tcpflood -B -I "${RSYSLOG_DYNNAME}.input" +shutdown_when_empty +wait_shutdown + +if [ -f "$TRAVERSAL_OUT" ]; then + echo "FAIL: dynafile traversal with default output template created: $TRAVERSAL_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +if [ ! -f "$VALID_OUT" ]; then + echo "FAIL: valid dynafile message after blocked traversal did not create: $VALID_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +rm -rf "$TRAVERSAL_ROOT" +exit_test diff --git a/tests/omfile-dynafile-traversal-list-template.sh b/tests/omfile-dynafile-traversal-list-template.sh new file mode 100755 index 000000000..83622bf0e --- /dev/null +++ b/tests/omfile-dynafile-traversal-list-template.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# Check dynafile traversal when the dynafile template is a list template. +. ${srcdir:=.}/diag.sh init +require_plugin imtcp + +export TRAVERSAL_APP="${RSYSLOG_DYNNAME}-omfile-list-template-traversal" +export TRAVERSAL_ROOT="${RSYSLOG_DYNNAME}.jail" +export TRAVERSAL_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.log" +rm -rf "$TRAVERSAL_ROOT" +mkdir -p "${TRAVERSAL_ROOT}/base" "${TRAVERSAL_ROOT}/escape" + +generate_conf +add_conf ' +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") +template(name="dynfile" type="list") { + constant(value="'$TRAVERSAL_ROOT'/base/") + property(name="hostname") + constant(value="/") + property(name="app-name") + constant(value=".log") +} +template(name="outfmt" type="string" string="%msg%\n") + +$rulesetparser rsyslog.rfc5424 +local4.debug action(type="omfile" dynafile="dynfile" template="outfmt") +' + +startup +printf '<167>1 2003-03-01T01:00:00.000Z ../escape %s - - - traversal-list-template\n' \ + "$TRAVERSAL_APP" > "${RSYSLOG_DYNNAME}.input" +tcpflood -B -I "${RSYSLOG_DYNNAME}.input" +shutdown_when_empty +wait_shutdown + +if [ -f "$TRAVERSAL_OUT" ]; then + echo "FAIL: list dynafile traversal created output outside template base: $TRAVERSAL_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +rm -rf "$TRAVERSAL_ROOT" +exit_test diff --git a/tests/omfile-dynafile-traversal-permit-escape.sh b/tests/omfile-dynafile-traversal-permit-escape.sh new file mode 100755 index 000000000..6e3d60c6f --- /dev/null +++ b/tests/omfile-dynafile-traversal-permit-escape.sh @@ -0,0 +1,54 @@ +#!/bin/bash +# Ensure the dangerous dynafile path escape override is explicit. +. ${srcdir:=.}/diag.sh init +require_plugin imtcp + +export TRAVERSAL_APP="${RSYSLOG_DYNNAME}-omfile-permit-path-escape" +export TRAVERSAL_ROOT="${RSYSLOG_DYNNAME}.jail" +export TRAVERSAL_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.log" +export PROTECTED_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.protected.log" +rm -rf "$TRAVERSAL_ROOT" +mkdir -p "${TRAVERSAL_ROOT}/base" "${TRAVERSAL_ROOT}/base-protected" "${TRAVERSAL_ROOT}/escape" + +generate_conf +add_conf ' +module(load="builtin:omfile") +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") +template(name="dynfile" type="string" string="'$TRAVERSAL_ROOT'/base/%HOSTNAME%/%APP-NAME%.log") +template(name="dynfile_protected" type="string" string="'$TRAVERSAL_ROOT'/base-protected/%HOSTNAME%/%APP-NAME%.protected.log") +template(name="outfmt" type="string" string="%msg%\n") + +$rulesetparser rsyslog.rfc5424 +local4.debug action(type="omfile" dynafile="dynfile" template="outfmt" + dynafile.dangerousPermitPathEscape="on") +local4.debug action(type="omfile" dynafile="dynfile_protected" template="outfmt") +' + +../tools/rsyslogd -C -N1 -M"$RSYSLOG_MODDIR" -f"${TESTCONF_NM}.conf" \ + >"${RSYSLOG_DYNNAME}.log" 2>&1 +if [ $? -ne 0 ]; then + echo "FAIL: action-level dangerous dynafile path escape warning rejected the configuration" + cat "${RSYSLOG_DYNNAME}.log" + error_exit 1 +fi + +startup +printf '<167>1 2003-03-01T01:00:00.000Z ../escape %s - - - permit-path-escape\n' \ + "$TRAVERSAL_APP" > "${RSYSLOG_DYNNAME}.input" +tcpflood -B -I "${RSYSLOG_DYNNAME}.input" +shutdown_when_empty +wait_shutdown + +if [ ! -f "$TRAVERSAL_OUT" ]; then + echo "FAIL: dangerous dynafile path escape override did not create $TRAVERSAL_OUT" + error_exit 1 +fi + +if [ -f "$PROTECTED_OUT" ]; then + echo "FAIL: per-action dangerous dynafile path escape leaked into protected action: $PROTECTED_OUT" + error_exit 1 +fi + +rm -rf "$TRAVERSAL_ROOT" +exit_test diff --git a/tests/omfile-dynafile-traversal-repro.sh b/tests/omfile-dynafile-traversal-repro.sh new file mode 100755 index 000000000..d4d6ad8ce --- /dev/null +++ b/tests/omfile-dynafile-traversal-repro.sh @@ -0,0 +1,37 @@ +#!/bin/bash +# Reproduce dynafile path traversal through a message-derived hostname. +. ${srcdir:=.}/diag.sh init +require_plugin imtcp + +export TRAVERSAL_APP="${RSYSLOG_DYNNAME}-omfile-traversal" +export TRAVERSAL_ROOT="${RSYSLOG_DYNNAME}.jail" +export TRAVERSAL_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.log" +rm -rf "$TRAVERSAL_ROOT" +mkdir -p "${TRAVERSAL_ROOT}/base" "${TRAVERSAL_ROOT}/escape" + +generate_conf +add_conf ' +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") +template(name="dynfile" type="string" string="'$TRAVERSAL_ROOT'/base/%HOSTNAME%/%APP-NAME%.log") +template(name="outfmt" type="string" string="%msg%\n") + +$rulesetparser rsyslog.rfc5424 +local4.debug action(type="omfile" dynafile="dynfile" template="outfmt") +' + +startup +printf '<167>1 2003-03-01T01:00:00.000Z ../escape %s - - - traversal-repro\n' "$TRAVERSAL_APP" \ + > "${RSYSLOG_DYNNAME}.input" +tcpflood -B -I "${RSYSLOG_DYNNAME}.input" +shutdown_when_empty +wait_shutdown + +if [ -f "$TRAVERSAL_OUT" ]; then + echo "FAIL: dynafile traversal created output outside template base: $TRAVERSAL_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +rm -rf "$TRAVERSAL_ROOT" +exit_test diff --git a/tests/yaml-omfile-dynafile-traversal-list-template.sh b/tests/yaml-omfile-dynafile-traversal-list-template.sh new file mode 100755 index 000000000..6604e709d --- /dev/null +++ b/tests/yaml-omfile-dynafile-traversal-list-template.sh @@ -0,0 +1,91 @@ +#!/bin/bash +# Check dynafile traversal with a YAML-defined list template. +. ${srcdir:=.}/diag.sh init +require_plugin imtcp + +export TRAVERSAL_APP="${RSYSLOG_DYNNAME}-yaml-omfile-list-template-traversal" +export TRAVERSAL_ROOT="${RSYSLOG_DYNNAME}.jail" +export TRAVERSAL_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.log" +export PERMIT_OUT="${TRAVERSAL_ROOT}/escape/${TRAVERSAL_APP}.permit.log" +rm -rf "$TRAVERSAL_ROOT" +mkdir -p "${TRAVERSAL_ROOT}/base" "${TRAVERSAL_ROOT}/base-permit" "${TRAVERSAL_ROOT}/escape" + +generate_conf +add_conf ' +include(file="'${RSYSLOG_DYNNAME}'.yaml") +' + +cat > "${RSYSLOG_DYNNAME}.yaml" << YAMLEOF +modules: + - load: "../plugins/imtcp/.libs/imtcp" + +templates: + - name: dynfile + type: list + elements: + - constant: + value: "${TRAVERSAL_ROOT}/base/" + - property: + name: hostname + - constant: + value: "/" + - property: + name: app-name + - constant: + value: ".log" + - name: outfmt + type: string + string: "%msg%\n" + - name: dynfile_permit + type: list + elements: + - constant: + value: "${TRAVERSAL_ROOT}/base-permit/" + - property: + name: hostname + - constant: + value: "/" + - property: + name: app-name + - constant: + value: ".permit.log" + +rulesets: + - name: main + filter: 'local4.debug' + actions: + - type: omfile + dynafile: dynfile + template: outfmt + - type: omfile + dynafile: dynfile_permit + template: outfmt + dynafile.dangerousPermitPathEscape: "on" +YAMLEOF + +add_conf ' +input(type="imtcp" port="0" listenPortFileName="'${RSYSLOG_DYNNAME}'.tcpflood_port" + ruleset="main") +' + +startup +printf '<167>1 2003-03-01T01:00:00.000Z ../escape %s - - - traversal-yaml-list-template\n' \ + "$TRAVERSAL_APP" > "${RSYSLOG_DYNNAME}.input" +tcpflood -B -I "${RSYSLOG_DYNNAME}.input" +shutdown_when_empty +wait_shutdown + +if [ -f "$TRAVERSAL_OUT" ]; then + echo "FAIL: YAML list dynafile traversal created output outside template base: $TRAVERSAL_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +if [ ! -f "$PERMIT_OUT" ]; then + echo "FAIL: YAML action-level dangerous dynafile path escape did not create $PERMIT_OUT" + rm -rf "$TRAVERSAL_ROOT" + error_exit 1 +fi + +rm -rf "$TRAVERSAL_ROOT" +exit_test diff --git a/tools/omfile.c b/tools/omfile.c index ca87b6f29..22ed996a3 100644 --- a/tools/omfile.c +++ b/tools/omfile.c @@ -57,6 +57,7 @@ #include "syslogd-types.h" #include "srUtils.h" #include "template.h" +#include "action.h" #include "outchannel.h" #include "omfile.h" #include "cfsysline.h" @@ -80,6 +81,11 @@ MODULE_CNFNAME("omfile") static rsRetVal resetConfigVariables(uchar __attribute__((unused)) * pp, void __attribute__((unused)) * pVal); static rsRetVal normalizeDynaFileCacheSize(int *const pNewVal); +typedef struct pathComponent_s { + const char *ptr; + size_t len; +} pathComponent_t; + /* internal structures */ DEF_OMOD_STATIC_DATA; @@ -156,6 +162,9 @@ typedef struct _instanceData { pthread_mutex_t mutWrite; /**< guard against multiple instances writing to single file */ uchar *fname; /**< file or template name (display only) */ uchar *tplName; /**< name of assigned template */ + uchar *dynaFileBasePath; /**< normalized constant base path for dynafile templates */ + sbool bPermitDynaFilePathEscape; /**< permit dynafile paths outside the derived base */ + sbool bRestrictDynaFileTplType; /**< reject template types without an inspectable fixed prefix */ strm_t *pStrm; /**< our output stream */ short nInactive; /**< number of minutes not writen (STATIC files only) */ char bDynamicName; /**< 0 - static name, 1 - dynamic name (with properties) */ @@ -221,6 +230,289 @@ typedef struct wrkrInstanceData { instanceData *pData; } wrkrInstanceData_t; +static int pathIsMeaningfulBase(const uchar *const path) { + return path != NULL && path[0] != '\0' && ustrcmp(path, (uchar *)".") && ustrcmp(path, (uchar *)"/"); +} + +static int pathStartsWithParentRef(const uchar *const path) { + return path[0] == '.' && path[1] == '.' && (path[2] == '\0' || path[2] == '/'); +} + +/** + * @brief Normalize a path syntactically without touching the filesystem. + * + * omfile may create missing directories and files, so realpath(3) cannot be + * used here. This helper removes duplicate separators and "." components, and + * resolves standalone ".." path components lexically. Relative paths that still + * escape above their starting point keep leading ".." components so callers can + * reject them. + * + * @param path Path string to normalize. + * @param ppNorm Receives a newly allocated normalized path on success. + * @return RS_RET_OK on success, RS_RET_OUT_OF_MEMORY on allocation failure. + */ +static rsRetVal normalizePathLexically(const uchar *const path, uchar **const ppNorm) { + pathComponent_t *components = NULL; + uchar *norm = NULL; + const char *pszPath; + const char *p; + const char *start; + uchar *dst; + size_t lenPath; + size_t maxComponents; + size_t nComponents = 0; + size_t lenComponent; + size_t lenNorm; + size_t i; + int isAbsolute; + DEFiRet; + + assert(path != NULL); + assert(ppNorm != NULL); + + pszPath = (const char *)path; + lenPath = strlen(pszPath); + maxComponents = lenPath / 2 + 2; + isAbsolute = pszPath[0] == '/'; + + CHKmalloc(components = calloc(maxComponents, sizeof(pathComponent_t))); + + p = pszPath; + while (*p != '\0') { + while (*p == '/') { + ++p; + } + start = p; + while (*p != '\0' && *p != '/') { + ++p; + } + lenComponent = (size_t)(p - start); + + if (lenComponent == 0 || (lenComponent == 1 && start[0] == '.')) { + continue; + } + if (lenComponent == 2 && start[0] == '.' && start[1] == '.') { + if (nComponents > 0 && + !(components[nComponents - 1].len == 2 && components[nComponents - 1].ptr[0] == '.' && + components[nComponents - 1].ptr[1] == '.')) { + --nComponents; + } else if (!isAbsolute) { + components[nComponents].ptr = start; + components[nComponents].len = lenComponent; + ++nComponents; + } + continue; + } + + components[nComponents].ptr = start; + components[nComponents].len = lenComponent; + ++nComponents; + } + + if (nComponents == 0) { + CHKmalloc(norm = (uchar *)strdup(isAbsolute ? "/" : ".")); + FINALIZE; + } + + lenNorm = isAbsolute ? 1 : 0; + for (i = 0; i < nComponents; ++i) { + lenNorm += components[i].len; + if (i > 0) { + ++lenNorm; + } + } + + CHKmalloc(norm = malloc(lenNorm + 1)); + dst = norm; + if (isAbsolute) { + *dst++ = '/'; + } + for (i = 0; i < nComponents; ++i) { + if (i > 0) { + *dst++ = '/'; + } + memcpy(dst, components[i].ptr, components[i].len); + dst += components[i].len; + } + *dst = '\0'; + +finalize_it: + free(components); + if (iRet == RS_RET_OK) { + *ppNorm = norm; + } else { + free(norm); + } + RETiRet; +} + +static int normalizedPathIsBelowBase(const uchar *const normPath, const uchar *const normBase) { + const size_t lenBase = ustrlen(normBase); + + if (!ustrcmp(normPath, normBase)) { + return 1; + } + return !strncmp((const char *)normPath, (const char *)normBase, lenBase) && normPath[lenBase] == '/'; +} + +/** + * @brief Validate a rendered dynamic file name before it can be opened. + * + * If the dynafile template had a meaningful static directory prefix, the + * rendered path must normalize to that directory or a child of it. For templates + * where no useful base can be derived, absolute paths and leading parent + * traversal are rejected as a fallback. Installations that intentionally rely + * on those legacy forms can opt in with dangerousPermitPathEscape. + * + * @param pData omfile action instance data. + * @param newFileName Rendered dynafile path. + * @return RS_RET_OK if the path is acceptable, RS_RET_ERR if it is blocked. + */ +static rsRetVal validateDynaFilePath(instanceData *const pData, const uchar *const newFileName) { + uchar *normPath = NULL; + DEFiRet; + + assert(pData != NULL); + assert(newFileName != NULL); + + CHKiRet(normalizePathLexically(newFileName, &normPath)); + if (pathIsMeaningfulBase(pData->dynaFileBasePath)) { + if (!normalizedPathIsBelowBase(normPath, pData->dynaFileBasePath)) { + LogError(0, RS_RET_ERR, "omfile: dynafile path traversal blocked: '%s' resolves outside base '%s'", + newFileName, pData->dynaFileBasePath); + ABORT_FINALIZE(RS_RET_ERR); + } + } else if (normPath[0] == '/' || pathStartsWithParentRef(normPath)) { + LogError(0, RS_RET_ERR, "omfile: dynafile path traversal blocked: '%s'", newFileName); + ABORT_FINALIZE(RS_RET_ERR); + } + +finalize_it: + free(normPath); + RETiRet; +} + +/** + * @brief Derive the immutable base directory from a dynafile template. + * + * The trusted part of a dynafile template is the contiguous run of constant + * template entries before the first property field. The last slash in that + * trusted prefix defines the directory base that rendered file names may not + * escape. For example, `/var/log/%HOSTNAME%/%APP-NAME%.log` derives `/var/log`. + * + * Templates backed by string generators or subtree output are intentionally not + * inspected; in those cases runtime validation falls back to rejecting absolute + * paths and leading parent traversal. + * + * @param pData omfile action instance data to receive the normalized base. + * @param pTpl Resolved dynafile template. + * @return RS_RET_OK on success, RS_RET_OUT_OF_MEMORY on allocation failure. + */ +static rsRetVal deriveDynaFileBasePath(instanceData *const pData, const struct template *const pTpl) { + const struct templateEntry *entry; + uchar *prefix = NULL; + uchar *base = NULL; + uchar *normBase = NULL; + uchar *lastSlash; + size_t prefixLen = 0; + size_t baseLen; + uchar *dst; + DEFiRet; + + assert(pData != NULL); + + free(pData->dynaFileBasePath); + pData->dynaFileBasePath = NULL; + + if (pTpl == NULL || pTpl->pStrgen != NULL || pTpl->bHaveSubtree) { + FINALIZE; + } + + /* Only constants before the first field are trusted as admin-authored path. */ + for (entry = pTpl->pEntryRoot; entry != NULL && entry->eEntryType == CONSTANT; entry = entry->pNext) { + prefixLen += (size_t)entry->data.constant.iLenConstant; + } + if (prefixLen == 0) { + FINALIZE; + } + + CHKmalloc(prefix = malloc(prefixLen + 1)); + dst = prefix; + for (entry = pTpl->pEntryRoot; entry != NULL && entry->eEntryType == CONSTANT; entry = entry->pNext) { + memcpy(dst, entry->data.constant.pConstant, (size_t)entry->data.constant.iLenConstant); + dst += entry->data.constant.iLenConstant; + } + *dst = '\0'; + + /* The base is the deepest complete directory in the trusted prefix. */ + lastSlash = (uchar *)strrchr((const char *)prefix, '/'); + if (lastSlash == NULL) { + FINALIZE; + } + + baseLen = (size_t)(lastSlash - prefix); + if (baseLen == 0) { + baseLen = 1; + } + CHKmalloc(base = malloc(baseLen + 1)); + memcpy(base, prefix, baseLen); + base[baseLen] = '\0'; + + CHKiRet(normalizePathLexically(base, &normBase)); + if (pathIsMeaningfulBase(normBase)) { + pData->dynaFileBasePath = normBase; + normBase = NULL; + } + +finalize_it: + free(prefix); + free(base); + free(normBase); + RETiRet; +} + +/** + * @brief Reject dynafile templates whose fixed prefix cannot be inspected. + * + * Dynafile path hardening depends on inspecting the static template entries + * before the first message-derived field. Rsyslog string and list templates + * both use the template entry list and can be checked this way. Subtree and + * plugin/string-generator templates bypass that representation, so omfile + * cannot derive a trusted base directory from their configured form. + * + * @param pData omfile action instance data. + * @param pTpl Resolved dynafile template. + * @return RS_RET_OK if the template type is accepted, RS_RET_ERR otherwise. + */ +static rsRetVal checkDynaFileTemplateType(instanceData *const pData, const struct template *const pTpl) { + DEFiRet; + + assert(pData != NULL); + + if (pData->bRestrictDynaFileTplType && (pTpl == NULL || pTpl->pStrgen != NULL || pTpl->bHaveSubtree)) { + LogError(0, RS_RET_ERR, + "omfile: dynafile template '%s' uses a template type that cannot be " + "safely inspected for path traversal; use type=\"string\" or " + "type=\"list\", or set module parameter " + "dynafile.restrictTemplateType=\"off\" to use the legacy " + "template subject to the fallback path guards", + pData->fname); + ABORT_FINALIZE(RS_RET_ERR); + } + +finalize_it: + RETiRet; +} + +static void warnDynaFilePathEscapeEnabled(const char *const scope) { + parser_warnmsg( + "omfile: %s dynafile.dangerousPermitPathEscape permits dynafile " + "paths to escape the configured base directory. This is dangerous " + "with untrusted message properties and can overwrite any file " + "permitted by OS permissions.", + scope); +} + /** * @brief Module-global configuration settings. * @@ -262,6 +554,8 @@ struct modConfData_s { gid_t fileGID; gid_t dirGID; int bDynafileDoNotSuspend; + sbool bPermitDynaFilePathEscape; + sbool bRestrictDynaFileTplType; strm_compressionDriver_t compressionDriver; int compressionDriver_workers; sbool bAddLF; /**< default setting for addLF action parameter */ @@ -287,6 +581,8 @@ static struct cnfparamdescr modpdescr[] = { {"fileownernum", eCmdHdlrInt, 0}, {"filegroup", eCmdHdlrGID, 0}, {"dynafile.donotsuspend", eCmdHdlrBinary, 0}, + {"dynafile.dangerouspermitpathescape", eCmdHdlrBinary, 0}, + {"dynafile.restricttemplatetype", eCmdHdlrBinary, 0}, {"filegroupnum", eCmdHdlrInt, 0}, }; static struct cnfparamblk modpblk = {CNFPARAMBLK_VERSION, sizeof(modpdescr) / sizeof(struct cnfparamdescr), modpdescr}; @@ -321,6 +617,7 @@ static struct cnfparamdescr actpdescr[] = {{"dynafilecachesize", eCmdHdlrInt, 0} {"rotation.sizelimitcommand", eCmdHdlrString, 0}, {"rotation.sizelimitcommandpassfilename", eCmdHdlrBinary, 0}, {"template", eCmdHdlrGetWord, 0}, + {"dynafile.dangerouspermitpathescape", eCmdHdlrBinary, 0}, {"addlf", eCmdHdlrBinary, 0}}; static struct cnfparamblk actpblk = {CNFPARAMBLK_VERSION, sizeof(actpdescr) / sizeof(struct cnfparamdescr), actpdescr}; @@ -783,6 +1080,12 @@ static rsRetVal ATTR_NONNULL() ABORT_FINALIZE(RS_RET_ERR); } + /* Compatibility escape hatch: disabled by default because it permits + * message-controlled dynafile paths to leave the configured base. */ + if (!pData->bPermitDynaFilePathEscape) { + CHKiRet(validateDynaFilePath(pData, newFileName)); + } + pCache = pData->dynCache; /* first check, if we still have the current file */ @@ -1028,6 +1331,8 @@ BEGINbeginCnfLoad pModConf->fileGID = -1; pModConf->dirGID = -1; pModConf->bDynafileDoNotSuspend = 1; + pModConf->bPermitDynaFilePathEscape = 0; + pModConf->bRestrictDynaFileTplType = 0; pModConf->bAddLF = 1; ENDbeginCnfLoad @@ -1098,6 +1403,13 @@ BEGINsetModCnf loadModConf->fileGID = (int)pvals[i].val.d.n; } else if (!strcmp(modpblk.descr[i].name, "dynafile.donotsuspend")) { loadModConf->bDynafileDoNotSuspend = (int)pvals[i].val.d.n; + } else if (!strcmp(modpblk.descr[i].name, "dynafile.dangerouspermitpathescape")) { + loadModConf->bPermitDynaFilePathEscape = pvals[i].val.d.n; + if (loadModConf->bPermitDynaFilePathEscape) { + warnDynaFilePathEscapeEnabled("module parameter"); + } + } else if (!strcmp(modpblk.descr[i].name, "dynafile.restricttemplatetype")) { + loadModConf->bRestrictDynaFileTplType = pvals[i].val.d.n; } else { dbgprintf( "omfile: program error, non-handled " @@ -1206,6 +1518,7 @@ BEGINfreeInstance CODESTARTfreeInstance; free(pData->tplName); free(pData->fname); + free(pData->dynaFileBasePath); free(pData->pszSizeLimitCmd); if (pData->iCloseTimeout > 0) janitorDelEtry(pData->janitorID); if (pData->bDynamicName) { @@ -1234,6 +1547,18 @@ BEGINfreeWrkrInstance ENDfreeWrkrInstance +BEGINsetActionInfo + CODESTARTsetActionInfo; + if (pData->bDynamicName && pAction->iNumTpls > 1) { + iRet = checkDynaFileTemplateType(pData, pAction->ppTpl[1]); + if (iRet != RS_RET_OK) { + RETiRet; + } + iRet = deriveDynaFileBasePath(pData, pAction->ppTpl[1]); + } +ENDsetActionInfo + + BEGINtryResume CODESTARTtryResume; ENDtryResume @@ -1299,6 +1624,9 @@ ENDcommitTransaction static void setInstParamDefaults(instanceData *__restrict__ const pData) { pData->fname = NULL; pData->tplName = NULL; + pData->dynaFileBasePath = NULL; + pData->bPermitDynaFilePathEscape = loadModConf->bPermitDynaFilePathEscape; + pData->bRestrictDynaFileTplType = loadModConf->bRestrictDynaFileTplType; pData->fileUID = loadModConf->fileUID; pData->fileGID = loadModConf->fileGID; pData->dirUID = loadModConf->dirUID; @@ -1563,6 +1891,11 @@ BEGINnewActInst pData->bDynamicName = 1; } else if (!strcmp(actpblk.descr[i].name, "template")) { CHKmalloc(pData->tplName = (uchar *)es_str2cstr(pvals[i].val.d.estr, NULL)); + } else if (!strcmp(actpblk.descr[i].name, "dynafile.dangerouspermitpathescape")) { + pData->bPermitDynaFilePathEscape = pvals[i].val.d.n; + if (pData->bPermitDynaFilePathEscape) { + warnDynaFilePathEscapeEnabled("action parameter"); + } } else if (!strcmp(actpblk.descr[i].name, "sig.provider")) { CHKmalloc(pData->sigprovName = (uchar *)es_str2cstr(pvals[i].val.d.estr, NULL)); } else if (!strcmp(actpblk.descr[i].name, "cry.provider")) { @@ -1799,6 +2132,7 @@ BEGINqueryEtryPt CODEqueryEtryPt_STD_CONF2_QUERIES; CODEqueryEtryPt_STD_CONF2_setModCnf_QUERIES; CODEqueryEtryPt_STD_CONF2_OMOD_QUERIES; + CODEqueryEtryPt_SetActionInfo_IF_OMOD_QUERIES; CODEqueryEtryPt_doHUP ENDqueryEtryPt -- 2.43.0