OS command injection: data filenames from a file-list are passed to `rm -f` via `system()` (`tracer_data.F90`, `metdata.F90`)

[Champollion9012]

What happened?

Two CAM routines delete a "used" input data file by building a shell command out of the filename
and running it through a shell, with no sanitization. The filename comes from an external
file-names list (a plain text file the user/case points CAM at), so any shell metacharacter in a
listed name is executed.

1) src/chemistry/utils/tracer_data.F90 (advance_file, ~line 2501):

if( file%remove_trc_file ) then
   call getfil( file%curr_filename, loc_fname, 0 )
   ctmp = 'rm -f ' // trim(loc_fname)          ! filename concatenated into a shell command
   call shr_sys_system( ctmp, istat )          ! -> execute_command_line -> /bin/sh -c
end if

file%curr_filename is produced by incr_filename (tracer_data.F90:837), which opens
file%filenames_list and reads filenames line by line. remove_trc_file is set from the rmv_file
argument (namelist option, e.g. the prescribed-data "remove file" flags).

2) src/dynamics/fv/metdata.F90 (check_files, ~line 1278):

if( met_remove_file ) then
   call getfil( curr_filename, loc_fname, 0 )
   ctmp = 'rm -f ' // trim(loc_fname)
   call shr_sys_system( ctmp, istat )
end if

curr_filename is produced by incr_filename (metdata.F90:1306), which reads the namelist
met_filenames_list text file line by line (read(unit,fmt='(A)') line — a whole line, internal
spaces preserved). Gate: met_remove_file (namelist), used by CAM-FV specified-dynamics / meteorology
nudging.

shr_sys_system ($CIMEROOT/src/share/util/shr_sys_mod.F90) runs the string via
execute_command_line, i.e. through /bin/sh -c, so ;, `, $(…), &, |, > in a filename
are interpreted as commands.

Impact: arbitrary command execution as the user running CAM, triggered during a normal run. This is
a data-only issue — independent of the binary, ASLR, or memory safety. On shared HPC systems the
file-list and data files are frequently shared/copied between users, so a poisoned list is a realistic
lateral-movement vector.

What are the steps to reproduce the bug?

Minimal, faithful standalone (the exact getfil→'rm -f '//name→shr_sys_system chain, ifx, ~5 s):

ctmp = 'rm -f ' // 'x.nc; touch /tmp/PWNED; id > /tmp/PWNED_ID.txt #'
call shr_sys_system(ctmp, istat)
=> /tmp/PWNED created; /tmp/PWNED_ID.txt = uid=…(the run user)

In a full run: point filenames_list (tracer_data / prescribed data) or met_filenames_list (SD
metdata) at a text file whose one of the listed names is
realdata.nc; touch /tmp/PWNED; id>/tmp/PWNED_ID.txt #, enable the corresponding remove-file flag
(rmv_file / met_remove_file), and run to the point the file is "finished with." The injected
commands execute.

(The sibling injection in $CIMEROOT — shr_file_get/shr_file_put/shr_dmodel /bin/cp+rm from a
data-model stream descriptor — is reported separately to the CIME repo; same root cause, and it is proven
end-to-end on a deployed 512-rank cesm.exe.)

What CAM tag were you using?

Observed in the CAM version shipped with CESM 2.1.5. The 'rm -f '//filename → system pattern is long-standing; please confirm against current CAM main (the advance_file/check_files remove-file paths).

What machine were you running CAM on?

CISL machine (e.g. derecho)

What compiler were you using?

Intel

Path to a case directory, if applicable

No response

Will you be addressing this bug yourself?

Yes

Extra info

Suggested fix

• Do not build a shell command from a file-derived name. Delete with a no-shell call (a C unlink()
  binding, or an execute_command_line that cannot be reparsed by /bin/sh).
• Validate filenames read from filenames_list / met_filenames_list against a strict whitelist
  ([A-Za-z0-9._/:+-]); reject shell metacharacters and a leading -.
• Apply the same treatment to every shr_sys_system('… '//name) site.

[jimmielin]

Thanks for reporting. I think if you can build/run CESM and edit user_nl_cam you're already on the other side of the security boundary so there is no eop here, but I agree it should be fixed.

This is a note that this bug does not apply in CAM-SIMA because the "remove" feature was intentionally removed when porting tracer_data over to CAM-SIMA.

[cacraigucar]

@fvitt - are you okay with us removing the sections of code highlighted above in 1 and 2? Should we also remove the associated namelist settings?

[fvitt]

@fvitt - are you okay with us removing the sections of code highlighted above in 1 and 2? Should we also remove the associated namelist settings?

Yes and yes. Those are leftovers from the legacy MOZART model.
Thanks