Let's define two helpers strdupa_safe() + strndupa_safe() which do the
same as their non-safe counterparts, except that they abort if called
with allocations larger than ALLOCA_MAX.
This should ensure that all our alloca() based allocations are subject
to this limit.
afaics glibc offers three alloca() based APIs: alloca() itself,
strndupa() + strdupa(). With this we have now replacements for all of
them, that take the limit into account.
Our coding style dictates we should not clobber return parameters on
failure, and always initialize them on success. Do so here.
This changes getenv_path_list() to return ENXIO if the env var is not
set, which is similar to how we handle this in getenv_bool().
This drops debug logging from parse_env_extension_hierarchies(), since
it's done anyway in getenv_path_list()
Follow-up for: #18018
All callers of strv_env_replace() would free the argument on error.
So let's follow the same pattern as with strv_consume (and similar
naming) and unconditionally "use up" the argument.
So far, we would allow certain control characters (NL since
b4346b9a77, TAB since 6294aa76d8), but not others. Having
other control characters in environment variable *value* is expected and widely
used, for various prompts like $LESS, $LESS_TERMCAP_*, and other similar
variables. The typical environment exported by bash already contains a dozen or
so such variables, so programs need to handle them.
We handle then correctly too, for example in 'systemctl show-environment',
since 804ee07c13. But we would still disallow setting such variables
by the user, in unit file Environment= and in set-environment/import-environment
operations. This is unexpected and confusing and doesn't help with anything
because such variables are present in the environment through other means.
When printing such variables, 'show-environment' escapes all special
characters, so variables with control characters are plainly visible.
In other uses, e.g. 'cat -v' can be used in similar fashion. This would already
need to be done to suppress color codes starting with \[.
Note that we still forbid invalid utf-8 with this patch. (Control characters
are valid, since they are valid 7-bit ascii.) I'm not sure if we should do
that, but since people haven't been actually asking for invalid utf-8, and only
for control characters, and invalid utf-8 causes other issues, I think it's OK
to leave this unchanged.
Fixes#4446, https://gitlab.gnome.org/GNOME/gnome-session/-/issues/45.
This reverts commit b45c068dd8.
I think the idea was generally sound, but didn't take into account the
limitations of show-environment and how it is used. People expect to be able to
eval systemctl show-environment output in bash, and no escaping syntax is
defined for environment *names* (we only do escaping for *values*). We could
skip such problematic variables in 'systemctl show-environment', and only allow
them to be inherited directly. But this would be confusing and ugly.
The original motivation for this change was that various import operations
would fail. a4ccce22d9 changed systemctl to filter
invalid variables in import-environment.
https://gitlab.gnome.org/GNOME/gnome-session/-/issues/71 does a similar change
in GNOME. So those problematic variables should not cause failures, but just
be silently ignored.
Finally, the environment block is becoming a dumping ground. In my gnome
session 'systemctl show-environment --user' includes stuff like PWD, FPATH
(from zsh), SHLVL=0 (no idea what that is). This is not directly related to
variable names (since all those are allowed under the stricter rules too), but
I think we should start pushing people away from running import-environment and
towards importing only select variables.
https://github.com/systemd/systemd/pull/17188#issuecomment-708676511
There was some confusion about what POSIX says about variable names:
names shall not contain the character '='. For values to be portable
across systems conforming to POSIX.1-2008, the value shall be composed
of characters from the portable character set (except NUL and as
indicated below).
i.e. it allows almost all ASCII in variable names (without NUL and DEL and
'='). OTOH, it says that *utilities* use a smaller set of characters:
Environment variable names used by the utilities in the Shell and
Utilities volume of POSIX.1-2008 consist solely of uppercase letters,
digits, and the <underscore> ( '_' ) from the characters defined in
Portable Character Set and do not begin with a digit.
When enforcing variable names in environment blocks, we need to use this
first definition, so that we can propagate all valid variables.
I think having non-printable characters in variable names is too much, so
I took out the whitespace stuff from the first definition.
OTOH, when we use *shell syntax*, for example doing variable expansion,
it seems enough to support expansion of variables that the shell would allow.
Fixes#14878,
https://bugzilla.redhat.com/show_bug.cgi?id=1754395,
https://bugzilla.redhat.com/show_bug.cgi?id=1879216.
Whenever I see EXTRACT_QUOTES, I'm always confused whether it means to
leave the quotes in or to take them out. Let's say "unquote", like we
say "cunescape".
Let's be more careful with what we serialize: let's ensure we never
serialize strings that are longer than LONG_LINE_MAX, so that we know we
can read them back with read_line(…, LONG_LINE_MAX, …) safely.
In order to implement this all serialization functions are move to
serialize.[ch], and internally will do line size checks. We'd rather
skip a serialization line (with a loud warning) than write an overly
long line out. Of course, this is just a second level protection, after
all the data we serialize shouldn't be this long in the first place.
While we are at it also clean up logging: while serializing make sure to
always log about errors immediately. Also, (void)ify all calls we don't
expect errors in (or catch errors as part of the general
fflush_and_check() at the end.
Let's clean up the failure codepaths, by using _cleanup_.
This relies on the new behaviour of env_append() introduced in the
previous commit that guarantess the list always remains properly NULL
terminated
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
Previously we were a bit sloppy with the index and size types of arrays,
we'd regularly use unsigned. While I don't think this ever resulted in
real issues I think we should be more careful there and follow a
stricter regime: unless there's a strong reason not to use size_t for
array sizes and indexes, size_t it should be. Any allocations we do
ultimately will use size_t anyway, and converting forth and back between
unsigned and size_t will always be a source of problems.
Note that on 32bit machines "unsigned" and "size_t" are equivalent, and
on 64bit machines our arrays shouldn't grow that large anyway, and if
they do we have a problem, however that kind of overly large allocation
we have protections for usually, but for overflows we do not have that
so much, hence let's add it.
So yeah, it's a story of the current code being already "good enough",
but I think some extra type hygiene is better.
This patch tries to be comprehensive, but it probably isn't and I missed
a few cases. But I guess we can cover that later as we notice it. Among
smaller fixes, this changes:
1. strv_length()' return type becomes size_t
2. the unit file changes array size becomes size_t
3. DNS answer and query array sizes become size_t
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76745
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
The environment variables we've serialized can quite possibly contain
characters outside the set allowed by env_assignment_is_valid(). In
fact, my environment seems to contain a couple of these:
* TERMCAP set by screen contains a '\x7f' character
* BASH_FUNC_module%% variable has a '%' character in name
Strict check of environment variables name and value certainly makes sense for
unit files, but not so much for deserialization of values we already had
in our environment.
