conf-parser breaks lines at LINE_MAX with unintended consequences/security ramifications

[Fabian-Gruenbichler]

Submission type

• Bug report

systemd version the issue has been seen with

systemd 229
+PAM +AUDIT +SELINUX +IMA +APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ -LZ4 +SECCOMP +BLKID +ELFUTILS +KMOD -IDN

Used distribution

Debian unstable (systemd: 229-6)

Expected behaviour you didn't see

In case a unit file contains lines that are longer than LINE_MAX characters,

• those lines should be parsed completely anyway (by merging the individually read, LINE_MAX characters long parts before parsing) or
• the parser should parse the first LINE_MAX characters and ignore the rest (with an appropriate warning?) or
• the parser should ignore the whole line (with an appropriate warning) or
• the parser should abort with an error message.

Unexpected behaviour you saw

In case a unit file contains lines that are longer than LINE_MAX characters, such lines are silently split every LINE_MAX characters and the resulting segments are treated as individual lines and parsed as such.

Steps to reproduce the problem

Use systemd-run to execute a rather long command as a scope unit. By default (unless an explicit --description is passed) the command string will be used as description for the generated unit file. If the command is longer than LINE_MAX (minus the Description= prefix), this allows to insert an additional configuration directive into the unit file by carefully crafting the command line.

Executing the following command (the aaa is just padding)

systemd-run --scope --unit test /bin/echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaatest=abc

leads to a journal entry like this:
May 13 14:29:18 debian systemd[1]: [/run/systemd/system/test.scope.d/50-Description.conf:3] Unknown lvalue 'test' in section 'Unit'

Including a Requires=something.service or Conflicts=something.service allows to start or stop arbitrary additional services (which can result in DoS and other security issues). JoinNameSpaceOf also looks pretty problematic/abusable to me, as do the ConditionXXX directives with would enable information leakage to an attacker.

This is not limited to systemd-run, as the root cause is in https://github.com/systemd/systemd/blob/master/src/shared/conf-parser.c#L318-L395 , but since systemd-run is often used to execute commands in a resource-limiting fashion which might contain user input somewhere on the commandline and this parser behaviour is quite unexpected it seems especially problematic. Quoting and escaping the user input is only of limited value here because the command that is executed via systemd-run might also use the "xyz=abc" syntax for passing options, and with sufficient padding possibilities an attacker can move that part to the LINE_MAX break that happens in the parser.

This issue is easily mitigated (for systemd-run) by setting an explicit description (for users of systemd-run) or by cutting off the autogenerated description if none is set explicitly (somewhere in the code-path executed by systemd-run?). I'd rather (or additionally) see the parser itself fixed though - I don't know where else systemd unit files are generated in a way that offers an attack surface via the parser's strange line splitting behaviour.

Other information

BOOT_IMAGE=/boot/vmlinuz-4.5.0-2-amd64 root=UUID=5e1db0da-44cd-4bee-9cc4-016c964436b3 ro systemd.log_level=debug systemd.log_target=kmsg log_buf_len=1M quiet
#2099 was basically the same issue, but was just worked-around by changing the generator to not produce long unit file lines (instead of fixing the root cause in the parser).

dmesg.txt
messages.txt

[poettering]

Yeah, we should probably generate a warning at least. Silently proceeding with the rest of the line as a new line is indeed not a good idea.

[butterflya]

NixOS/nixpkgs#3403 also suffers from this limit.

Why can't you engineer systems such that they Just Work? Why are there limitations in the system? Why didn't you just make it such that people who work in resource constrained environments have to do extra work (like specifying an upper bound on various buffers of systemd) to obtain this instead of bothering people with 256GB RAM servers?

[DemiMarie]

@butterflya I don't know, but 256 GB servers are rare. I will say, though, that this limit is too small.

[poettering]

@butterflya well, we generally put limits on all resources, as a matter of writing safe code. Clients should not be able to reserve resources without bounds for their operations.

And I am pretty sure the existing code actually works for most people, using overly long lines is not precisely the most typical of cases. Note that that libc and POSIX actually acknowledge the need for limits, by defining LINE_MAX which we piggyback on here, so that we don't have to come up with out own limit.

I am pretty sure we should enforce a limit here. I am also pretty sure we should not silently consider overly long lines multiple lines, but log about this case. And I figure we can raise the limit. But I still think that there needs to be a limit enforced.

[DemiMarie]

What about continued lines? Should they count towards the limit?

On Aug 19, 2016 10:20 AM, "Lennart Poettering" notifications@github.com
wrote:

@butterflya https://github.com/butterflya well, we generally put limits
on all resources, as a matter of writing safe code. Clients should not be
able to reserve resources without bounds for their operations.

And I am pretty sure the existing code actually works for most people,
using overly long lines is not precisely the most typical of cases. Note
that that libc and POSIX actually acknowledge the need for limits, by
defining LINE_MAX which we piggyback on here, so that we don't have to come
up with out own limit.

I am pretty sure we should enforce a limit here. I am also pretty sure we
should not silently consider overly long lines multiple lines, but log
about this case. And I figure we can raise the limit. But I still think
that there needs to be a limit enforced.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3302 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGGWB4ABbipd7sj5ObVVV4sOfC4bT2Tbks5qhbvJgaJpZM4Ii614
.

[ptman]

Clear documentation and a warning would be a start