ESCOMP / ESCOMP/CAM

Integer overflow of 32-bit NetCDF dimension products → undersized allocation → heap out-of-bounds write in sat_hist.F90 and tools/mkatmsrffile

Open
#1,620 2 comments 0 reactions 0 assignees View on GitHub
misc tag
Dominant language
No language data
Stars
91
Forks
183
Avg merge
6d 2m
Merged PRs (30d)
9

Description

## Summary

Two CAM code paths read 64-bit NetCDF dimension lengths into **default (32-bit) Fortran integers**, multiply them to compute an allocation size, and then fill the resulting array with the *true* (64-bit) element count. When the dimension product exceeds `2**31`, the 32-bit product wraps to a small — or negative, and hence clamped to a zero/short — value. The allocation is therefore undersized while the subsequent write loop runs to the true length, producing a **heap out-of-bounds write** (CWE-190 → CWE-787). Both sites are reachable from an attacker-supplied / crafted NetCDF input file, so this is a **remotely-triggerable denial-of-service / heap-integrity violation**.

Neither site validates the dimension factors against a compiled-in maximum before multiplying.

---

## Affected code

### Site 1 — `src/control/sat_hist.F90` @ `cam_development`

The `profs` NetCDF dimension of the satellite-track file is read into the default-integer `n_profiles` (declared L62), which bounds `ncols`:

```fortran
! L62
integer :: n_profiles
...
! L216-217 (subroutine sat_hist_init, reading sathist_track_infile)
ierr = pio_inq_dimid(infile,'profs',dimid)
ierr = pio_inq_dimlen(infile, dimid, n_profiles)
```

`ncols` (default integer, L425) is derived from that count (`ncols = end_ndx-beg_ndx+1`, L713) and multiplied by the namelist value `sathist_nclosest` (default integer, L45) to size **nine** arrays:

```fortran
! L425
integer :: ncols, nocols
...
! L443
call read_next_position( ncols )
...
! L451-461
nocols = ncols * sathist_nclosest

allocate( col_ndxs(nocols) )
allocate( chk_ndxs(nocols) )
allocate( fdyn_ndxs(nocols) )
allocate( ldyn_ndxs(nocols) )
allocate( phs_owners(nocols) )
allocate( dyn_owners(nocols) )
allocate( mlats(nocols) )
allocate( mlons(nocols) )
allocate( phs_dists(nocols) )
```

The same wrapping product recurs in `dump_columns` (L762-763) and in `write_output` (`outdata`), where the fill loop indexes to the true count:

```fortran
! L986-990
allocate( outdata(ncols * sathist_nclosest) )

do i = 1, ncols
outdata(((i-1)*sathist_nclosest)+1 : (i*sathist_nclosest)) = data(i)
enddo
```

`sathist_nclosest` is a `satellite_options_nl` namelist variable (declared L45, in namelist L128-129, default `1` at L137) and is **not** range-checked. Abort routine in this file: `use cam_abortutils, only: endrun` (L14).

### Site 2 — `tools/mkatmsrffile/mkatmsrffile.F90` @ `cam_development`

The `grid_size` dimension of the atmosphere NetCDF file (`atmFileName`) is read into the default-integer `nxg`, divided to `nx`, and used to size `gindex`; `atmnx` (default integer, L52) — the same grid size returned via `openfile_and_initdecomp` (L97) — sizes `dof2`:

```fortran
! L52
integer :: srfnx, atmnx, srfnxg, atmnxg, dimid, nlat, nlon, i, j, clen, index, dim1, dim2
...
! L294-299
allocate(dof2(atmnx*12))
do j=1,12
do i=1,atmnx
dof2(i+(j-1)*atmnx) = dof(i)+(j-1)*atmnxg
end do
end do
...
! L326 / L346 (get_grid_index: nx, nxg are default integer)
integer, intent(out) :: nx, nxg
...
! L356-368
ierr = pio_inq_dimid(File, 'grid_size', dimid)
ierr = pio_inq_dimlen(File, dimid, nxg)

nx = nxg/npes
...
allocate(gindex(nx+add1))
do i=1,nx+add1
gindex(i)=i+iam*nx+start_offset
end do
```

`atmnx*12` and `nxg` are default 32-bit throughout with no bounds check. This file currently has **no** abort routine imported (it already `use`s `shr_kind_mod` from CIME share code, so `shr_sys_abort` is available).

---

## Root cause

`pio_inq_dimlen` returns a 64-bit on-disk dimension length, but the receiving variables (`n_profiles`, `nxg`, and the derived `ncols`, `atmnx`) are **default `integer` (32-bit)**. The allocation size is formed as a 32-bit product:

- `nocols = ncols * sathist_nclosest`
- `atmnx*12`, `nx+add1`

Once the true product exceeds the signed 32-bit range (`2**31`), the multiply wraps modulo `2**32`, yielding a small positive, or a negative (→ effectively zero-length) allocation. The fill loops, however, iterate over the true element count, so every element past the wrapped size is written **out of bounds** on the heap.

---

## Trigger / attack vector

- **Site 1:** A crafted satellite-track NetCDF referenced by `sathist_track_infile` (`satellite_options_nl`) with a large `profs` dimension, combined with a large `sathist_nclosest`, such that `ncols * sathist_nclosest` overflows 32-bit. `sathist_nclosest` is operator-controlled and unbounded; `ncols`/`profs` come directly from the input file.
- **Site 2:** A crafted atmosphere NetCDF (`atmFileName`, the `mkatmsrffile` offline tool) with a `grid_size` dimension whose value (or `grid_size*12`) overflows 32-bit.

In both cases the malicious value lives entirely in a NetCDF input file, so the crash is reachable without any code change on the target.

---

## Impact

- **Classification:** CWE-190 (integer overflow) → CWE-787 (out-of-bounds write).
- **Severity effect:** remotely-triggerable **denial of service / heap-integrity violation** from a crafted input file.
- **Not RCE:** the overflow deficit is structurally unbounded (≥ `2**31` elements), so the contiguous fill loop runs off the end of mapped memory and the process crashes *during* the loop, before any overwritten heap metadata or pointer could be dereferenced — there is no controlled-write primitive an attacker could steer toward code execution.

---

## Reproduction

Construct a NetCDF file whose relevant dimensions multiply to just over `2**32`:

- Site 1: `profs` (→ `ncols`) × `sathist_nclosest` slightly above `2**32` (e.g. `ncols ≈ 2.2e9`, or a modest `ncols` with a large namelist `sathist_nclosest`).
- Site 2: `grid_size` such that `grid_size*12` slightly exceeds `2**32`.

The 32-bit product wraps to a small value, `allocate` under-sizes the buffer, and the fill loop writes past it. A minimal harness (`ifx -fsanitize=address`) that mirrors the `sat_hist.F90` structure exactly —
```fortran
nocols = ncols * sathist_nclosest ! == L451, 32-bit product
allocate( outdata(nocols) ) ! == L986
do i = 1, ncols
outdata( (i-1)*sathist_nclosest+1 : i*sathist_nclosest ) = i ! == L988-990
end do
```
— reproduces it deterministically with `ncols=65536, sathist_nclosest=65537` (true product `4,295,032,832` → wraps to `65536`):
```
[sat_hist] nocols(int32)=65536
[sat_hist] allocated outdata(65536)
==ERROR: AddressSanitizer: heap-buffer-overflow WRITE of size 4 at MAIN ... :19
0x... is located 0 bytes after 262184-byte region
SUMMARY: AddressSanitizer: heap-buffer-overflow ... in MAIN
```
i.e. the first write past the wrapped allocation is flagged at the fill loop. (Build: `ifx -O0 -g -fsanitize=address`, glibc 2.38.)

---

## Suggested fix

Preferred: bound each dimension factor against a compiled-in maximum **immediately after it is read**, aborting via the component's standard routine so the product can never overflow. (Equivalently, compute the product in `integer(8)` and validate before `allocate`.)

### `src/control/sat_hist.F90`

```fortran
! module-scope parameter (choose a value well above any realistic size,
! but far below the 32-bit limit so the product cannot wrap)
integer, parameter :: max_profs = 100000000 ! 1e8 satellite profiles
integer, parameter :: max_nclosest = 100000

! ---- immediately after L217, having read the 'profs' dimension ----
ierr = pio_inq_dimlen(infile, dimid, n_profiles)
if (n_profiles < 0 .or. n_profiles > max_profs) then
call endrun('sat_hist: profs dimension in '//trim(locfn)// &
' is out of range (possible 32-bit integer overflow)')
end if

! ---- after the namelist read / mpibcast of sathist_nclosest ----
if (sathist_nclosest < 1 .or. sathist_nclosest > max_nclosest) then
call endrun('sat_hist: sathist_nclosest out of range')
end if

! ---- belt-and-suspenders at the product site (replaces L451) ----
if (int(ncols,8) * int(sathist_nclosest,8) > int(huge(nocols),8)) then
call endrun('sat_hist: ncols*sathist_nclosest overflows 32-bit integer')
end if
nocols = ncols * sathist_nclosest
```

### `tools/mkatmsrffile/mkatmsrffile.F90`

```fortran
use shr_sys_mod, only : shr_sys_abort ! add to the use list

integer, parameter :: max_grid_size = 100000000

! ---- immediately after L357, having read 'grid_size' ----
ierr = pio_inq_dimlen(File, dimid, nxg)
if (nxg < 0 .or. nxg > max_grid_size) then
call shr_sys_abort('mkatmsrffile: grid_size out of range in '//trim(filename)// &
' (possible 32-bit integer overflow)')
end if
```

The `int(...,8)` guard (or a compiled-in `max_*` bound) is sufficient at each site; the two together are belt-and-suspenders.

---

## Severity

**Medium** — remotely-triggerable crash / heap corruption from a crafted NetCDF input; no privilege escalation or code execution.

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.