This would break third-parties like https://github.com/garrettj403/SciencePlots which explicitly rely on those settings being available in style files.
I have argued elsewhere that I don't believe in neither the style file system (they should just be python modules that expose a rcparams_update dict or function) nor the security model around them (ipython is happy with plain executable config files), so I don't think I can really judge this, because I simply don't understand the point.

I would be curious how you could do that, given that \usepackage{foo} can basically run anything (whatever is in foo.sty, including \write18-based tex shell escapes).

I do not think this is currently exploitable. Per https://latexref.xyz/_005cwrite18.html (TBD is that is the best reference) and https://latexref.xyz/Command-line-options.html (which matches the help on my machine)

 [belanna] @ latex --help
<snip>
[-no]-shell-escape      disable/enable \write18{SHELL COMMAND}
-shell-restricted       enable restricted \write18
<snip>

there is no ENV to control this and to enable shell commands you must pass --shell-escape to the command line invocation of latex. We control that (with currently no options to add user supplied flags.

In principle someone could add their own latex to the PATH ahead of the system one which turns this on, but if the attacker already has the ability to write arbitrary files and to modify PATH, they are already on the wrong side of the airlock so we are not the problem.

Per all of that, I do not think we need to make this change.

I think the main thing we should explicitly add --no-shell-escape to our latex invocations to be safe.

I tested that if you add flags to turn it off and on then the last one wins so even if the user has gone and made an alias/helper that turns it on, we will still turn it off unless they go to the effort processing the command line arguments to eject the --no-shell-escape or put theirs last....and at that point I'm a bit out of pity and we should let that user do what they want.

We may also want to add a disable_preamble which is excluded from the style files that is used to filter them which defaults to False (current behavior). That way people can turn it off if they want, but I don't have strong feelings either way.

If I am wrong about the above, then we need to find a better way to handle the _classic style as it needs to be able to remove any preamble the user has set.

The issue is that default shell access can be set as a system level config, and we don't control that. That's set by shell_escape = t in the system texmf.cnf. Is this common? Probably not, but I do see it recommended a good number of times when googling. Relevant TeX docs: https://tug.org/texinfohtml/web2c.html#Shell-escapes

"In principle someone could add their own latex to the PATH " - I'm not really following this? We don't package our own latex?

I like the idea of solving this by putting --no-shell-escape at the end of all our latex calls instead. Should override the system level config and it seems like the preambles are useful.

Ok, now knowing that there in texmf.cnf I'm 100% on us adding --no-shell-escape to our invocations (as in we should do it for 3.11) and like like 75% on adding the "turn off preamble"

(justification for the above)

Polling the two distros I have easy access to (arch and RHEL9), the they both have

% Enable system commands via \write18{...}.  When enabled fully (set to
% t), obviously insecure.  When enabled partially (set to p), only the
% commands listed in shell_escape_commands are allowed.  Although this
% is not fully secure either, it is much better, and so useful that we
% enable it for everything but bare tex.
shell_escape = p

with slightly different allowed programs

% No spaces in this command list.
%
% The programs listed here are as safe as any we know: they either do
% not write any output files, respect openout_any, or have hard-coded
% restrictions similar to or higher than openout_any=p.  The output file
% location is determined according to
% https://tug.org/texinfohtml/web2c.html#Output-file-location.
%
% They also have no features to invoke arbitrary other programs, and no
% known exploitable bugs, to the best of our knowledge.
%
% Finally, they also have practical use for being called from TeX.
%
shell_escape_commands = \
bibtex,bibtex8,\
extractbb,\
gregorio,\
kpsewhich,\
l3sys-query,\
latexminted,\
makeindex,\
memoize-extract.pl,\
memoize-extract.py,\
repstopdf,\
r-mpost,\
texosquery-jre8,\

% No spaces in this command list.
%
% The programs listed here are as safe as any we know: they either do
% not write any output files, respect openout_any, or have hard-coded
% restrictions similar to or higher than openout_any=p.  They also have
% no features to invoke arbitrary other programs, and no known
% exploitable bugs.  All to the best of our knowledge.  They also have
% practical use for being called from TeX.
%
shell_escape_commands = \
bibtex,bibtex8,\
extractbb,\
gregorio,\
kpsewhich,\
makeindex,\
repstopdf,\
r-mpost,\
texosquery-jre8,\

in the system install texmf.cnf. It looks like the upstream base configuration file is https://github.com/TeX-Live/texlive-source/blob/51b51010d300bb364325471fc1d72d85f7b2430f/texk/kpathsea/texmf.cnf#L634-L639 which is also set to 'p' (which means only the commands in the allow list which per the pdfTeX developers are safe).

So my analysis is changed a bit in that currently a user or a packager could have set things up in a way to make this exploitable, but they would have had to intentionally do that. We do not want to drop the preamble as it is useful (and we have at least one know use in the wild), but we do want to force --no-shell-escape on our invocations as a precaution.

details on why the above is not complete too-clever-user proof

I mean if someone drops an executable called latex in their path with the contents

#!/bin/bash
exec latex --shell-escape "$@"

or the pathological version

#!/bin/bash
args=()
for arg in "$@"; do
    [[ "$arg" == "--no-shell-escape" ]] && continue
    args+=("$arg")
done
exec latex --shell-escape "${args[@]}"

(to make these really work you would have find the actual real latex, but that is left as an exercise for those making poor life choices)

someplace in the system PATH ahead of real latex (I know the *nix way of doing this, I understand it is different in windows, but nothing past that), then they can control what cli flags latex sees and force escaping to happen. But from a security point of view, if the attacker can already write files to disk and modify $PATH, then they already have the ability to write files and modify the $PATH which makes doing so via injecting arbitrary code into the preamble redundant. And if the user is this dedicated to making arbitrary shell work in their preamble I wish them good luck 🤣 .

Shell escape changes opened in #31282

IMO that's enough to keep the preamble and close this PR. That still leaves open the possibility of reading / writing files, but not arbitrary code execution afaict. The default openout_any = p config restricts TeX to writing to the current directory.

Yeah, I don't think covering for PATH modifications is a meaningful security goal. A malicious matplotlibrc file is already a bit of a stretch since it requires a download, but is more plausibly overlooked as a risk.

Talked on the call today and agreed that the -no-shell-escape flag is enough to mitigate this vulnerability. We won't add a switch at this time to disable that flag - if this breaks someone else's tooling we can discuss the particulars then.