ESCOMP / ESCOMP/CAM

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

Open
#1,606 3 comments 0 reactions 1 assignee Claimed by @cacraigucar View on GitHub
bug misc tag
Dominant language
No language data
Stars
91
Forks
183
Avg merge
6d 2m
Merged PRs (30d)
9

Description

### 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):**
```fortran
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):**
```fortran
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.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.