Skip to content

Commit 9bdcede

Browse files
jim-p-wjim-p-w
committed
Add a test to verify reading character variables won't overrun buffers.
Character variables are read into fixed size arrays when reading netcdf files. A test is added which tries to read character variables into a buffer which isn't large enough to hold the data. The test verifies the read fails with a suitable error code.
1 parent e269673 commit 9bdcede

File tree

3 files changed

+216
-2
lines changed

3 files changed

+216
-2
lines changed

‎src/core_test/Makefile‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ OBJS = mpas_test_core.o \
1212
mpas_test_core_dmpar.o \
1313
mpas_test_core_stream_inquiry.o \
1414
mpas_test_openacc.o \
15-
mpas_test_core_stream_list.o
15+
mpas_test_core_stream_list.o \
16+
mpas_test_core_io.o
1617

1718
all: core_test
1819

@@ -44,7 +45,7 @@ mpas_test_core.o: mpas_test_core_halo_exch.o mpas_test_core_streams.o \
4445
mpas_test_core_sorting.o mpas_halo_testing.o \
4546
mpas_test_core_string_utils.o mpas_test_core_dmpar.o \
4647
mpas_test_core_stream_inquiry.o mpas_test_openacc.o \
47-
mpas_test_core_stream_list.o
48+
mpas_test_core_stream_list.o mpas_test_core_io.o
4849

4950
mpas_test_core_halo_exch.o:
5051

‎src/core_test/mpas_test_core.F‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ function test_core_run(domain) result(iErr)!{{{
9797
use test_core_string_utils, only : mpas_test_string_utils
9898
use mpas_test_core_dmpar, only : mpas_test_dmpar
9999
use mpas_test_core_stream_inquiry, only : mpas_test_stream_inquiry
100+
use test_core_io, only : test_core_io_test
100101
use mpas_test_core_openacc, only : mpas_test_openacc
101102

102103
implicit none
@@ -224,6 +225,18 @@ function test_core_run(domain) result(iErr)!{{{
224225

225226
call mpas_stream_mgr_write(domain % streamManager, forceWriteNow=.true.)
226227

228+
!
229+
! Run io tests
230+
!
231+
call mpas_log_write('')
232+
call test_core_io_test(domain, iErr)
233+
if (iErr == 0) then
234+
call mpas_log_write('All tests PASSED')
235+
else
236+
call mpas_log_write('$i tests FAILED', intArgs=[iErr])
237+
end if
238+
call mpas_log_write('')
239+
227240
!
228241
! Run mpas_test_openacc
229242
!

‎src/core_test/mpas_test_core_io.F‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
! Copyright (c) 2025 The University Corporation for Atmospheric Research (UCAR).
2+
!
3+
! Unless noted otherwise source code is licensed under the BSD license.
4+
! Additional copyright and license information can be found in the LICENSE file
5+
! distributed with this code, or at https://mpas-dev.github.io/license.html .
6+
!
7+
module test_core_io
8+
9+
#define ERROR_WRITE(M) call mpas_log_write( M , messageType=MPAS_LOG_ERR)
10+
#define ERROR_WRITE_ARGS(M, ARGS) call mpas_log_write( M , ARGS, messageType=MPAS_LOG_ERR)
11+
use mpas_log
12+
use mpas_io
13+
14+
implicit none
15+
private
16+
public :: test_core_io_test
17+
18+
contains
19+
20+
!***********************************************************************
21+
!
22+
! routine close_file_with_message
23+
!
24+
!> \brief closes the provided file handle and writes an error message.
25+
!-----------------------------------------------------------------------
26+
subroutine close_file_with_message(fileHandle, message, args)
27+
type(MPAS_IO_Handle_type), intent(inout) :: fileHandle
28+
character (len=*), intent(in), optional :: message
29+
integer, dimension(:), intent(in), optional :: args
30+
31+
integer :: local_ierr
32+
33+
! log an error message
34+
if (present(message)) then
35+
ERROR_WRITE_ARGS(message, intArgs=args)
36+
end if
37+
38+
! close the provided file
39+
call MPAS_io_close(fileHandle, local_ierr)
40+
if (local_ierr /= MPAS_IO_NOERR) then
41+
ERROR_WRITE_ARGS('MPAS_io_close failed with error code:$i', intArgs=(/local_ierr/))
42+
return
43+
endif
44+
45+
end subroutine close_file_with_message
46+
47+
!***********************************************************************
48+
!
49+
! routine test_read_string_buffer_check
50+
!
51+
!> \brief verifies attempts to read strings into buffers which are too small
52+
!> to hold the value fails safely.
53+
!> \details
54+
!> Run these tests with valgrind to ensure there are no buffer overflows when
55+
!> attempting to read strings into undersized buffers.
56+
!-----------------------------------------------------------------------
57+
subroutine test_read_string_buffer_check(domain, ierr)
58+
59+
type (domain_type), intent(inout) :: domain
60+
integer, intent(out) :: ierr
61+
62+
integer :: local_ierr, i
63+
type(MPAS_IO_Handle_type) :: fileHandle
64+
character (len=StrKIND), dimension(1), parameter :: dimNamesString = ['StrLen']
65+
character (len=StrKIND), dimension(2), parameter :: dimNamesStringTime = &
66+
[character(len=StrKIND) :: 'StrLen', 'Time']
67+
character (len=32), parameter :: varName1 = 'stringVar'
68+
character (len=32), parameter :: varName2 = 'stringTimeVar'
69+
character (len=*), parameter :: varValue1 = 'This is a string'
70+
character (len=32), dimension(2), parameter :: varNames = [varName1, varName2]
71+
integer, parameter :: bufferSize=128
72+
integer, parameter :: smallBufferSize=bufferSize/2
73+
character (len=bufferSize) :: buffer
74+
character (len=smallBufferSize) :: smallBuffer
75+
character (len=*), parameter :: filename = 'char_data.nc'
76+
77+
ierr = 0
78+
79+
! open a file to write char variables to
80+
fileHandle = MPAS_io_open(filename, MPAS_IO_WRITE, MPAS_IO_NETCDF, domain % ioContext, &
81+
clobber_file=.true., truncate_file=.true., ierr=local_ierr)
82+
if (local_ierr /= MPAS_IO_NOERR) then
83+
ierr = 1
84+
ERROR_WRITE('Error opening file ' // trim(filename))
85+
return
86+
end if
87+
88+
! define dimensions and char variables
89+
call MPAS_io_def_dim(fileHandle, dimNamesStringTime(1), bufferSize, local_ierr)
90+
if (local_ierr /= MPAS_IO_NOERR) then
91+
ierr = 1
92+
call close_file_with_message(fileHandle, 'Error defining '//trim(dimNamesStringTime(1))//', error=$i', (/local_ierr/))
93+
return
94+
end if
95+
call MPAS_io_def_dim(fileHandle, dimNamesStringTime(2), MPAS_IO_UNLIMITED_DIM, local_ierr)
96+
if (local_ierr /= MPAS_IO_NOERR) then
97+
ierr = 1
98+
call close_file_with_message(fileHandle, 'Error defining '//trim(dimNamesStringTime(2))//', error=$i', (/local_ierr/))
99+
return
100+
end if
101+
call MPAS_io_def_var(fileHandle, varNames(1), MPAS_IO_CHAR, dimNamesString, ierr=local_ierr)
102+
if (local_ierr /= MPAS_IO_NOERR) then
103+
ierr = 1
104+
call close_file_with_message(fileHandle, 'Error defining var "'//trim(varNames(1))//'" error=$i', (/local_ierr/))
105+
return
106+
end if
107+
call MPAS_io_def_var(fileHandle, varNames(2), MPAS_IO_CHAR, dimNamesStringTime, ierr=local_ierr)
108+
if (local_ierr /= MPAS_IO_NOERR) then
109+
ierr = 1
110+
call close_file_with_message(fileHandle, 'Error defining var "'//trim(varNames(2))//'" error=$i', (/local_ierr/))
111+
return
112+
end if
113+
114+
! write the string values
115+
do i=1,size(varNames)
116+
call MPAS_io_put_var_char0d(fileHandle, varNames(i), varValue1, local_ierr)
117+
if (local_ierr /= MPAS_IO_NOERR) then
118+
ierr = 1
119+
call close_file_with_message(fileHandle, 'Error writing "'//trim(varNames(i))// &
120+
'", error=$i', (/local_ierr/))
121+
return
122+
end if
123+
124+
! verify the strings are read into buffers which are large enough for the string values
125+
call MPAS_io_get_var_char0d(fileHandle, varNames(i), buffer, local_ierr)
126+
if (local_ierr /= MPAS_IO_NOERR) then
127+
ierr = 1
128+
call close_file_with_message(fileHandle, 'Error reading "'//trim(varNames(i))// &
129+
'", error=$i', (/local_ierr/))
130+
return
131+
end if
132+
end do
133+
134+
! verify attempts to read strings into buffers which are too small generates an error
135+
call mpas_log_write(' ')
136+
call mpas_log_write('Expect to see the following error:')
137+
call MPAS_io_err_mesg(domain % ioContext, MPAS_IO_ERR_INSUFFICIENT_BUF, .false.)
138+
call mpas_log_write(' ')
139+
do i=1,size(varNames)
140+
! this should return an error
141+
call MPAS_io_get_var_char0d(fileHandle, varNames(i), smallBuffer, local_ierr)
142+
call mpas_log_write(' ')
143+
144+
if (local_ierr /= MPAS_IO_ERR_INSUFFICIENT_BUF) then
145+
ierr = 1
146+
if (local_ierr == MPAS_IO_NOERR) then
147+
call close_file_with_message(fileHandle, 'Expected MPAS_IO_ERR_INSUFFICIENT_BUF ($i)'&
148+
//' but recieved no error reading "'//trim(varName1), (/local_ierr/))
149+
else
150+
call close_file_with_message(fileHandle, 'Expected MPAS_IO_ERR_INSUFFICIENT_BUF ($i)'&
151+
//' but recieved error $i reading "'//trim(varName1)//'"', &
152+
(/MPAS_IO_ERR_INSUFFICIENT_BUF, local_ierr/))
153+
end if
154+
return
155+
end if
156+
end do
157+
call close_file_with_message(fileHandle)
158+
159+
end subroutine test_read_string_buffer_check
160+
161+
162+
!***********************************************************************
163+
! Subroutine test_core_io_test
164+
!
165+
!> \brief Core test suite for I/O
166+
!>
167+
!> \details This subroutine tests mpas_io features.
168+
!> It calls individual tests for I/O operations.
169+
!> See the subroutine body for details.
170+
!> The results of each test are logged with a success or failure message.
171+
!>
172+
!> \param domain The domain object that contains the I/O context
173+
!> \param ierr The error code that indicates the result of the test.
174+
!
175+
!-----------------------------------------------------------------------
176+
subroutine test_core_io_test(domain, ierr)
177+
178+
use mpas_log
179+
180+
type (domain_type), intent(inout) :: domain
181+
integer, intent(out) :: ierr
182+
183+
integer :: test_status
184+
185+
ierr = 0
186+
test_status = 0
187+
188+
call mpas_log_write('Testing char-0 buffer reads')
189+
call test_read_string_buffer_check(domain, test_status)
190+
if (test_status == 0) then
191+
call mpas_log_write('char-0 buffer tests: SUCCESS')
192+
else
193+
call mpas_log_write('char-0 buffer tests: FAILURE', MPAS_LOG_ERR)
194+
ierr = ierr + abs(test_status)
195+
end if
196+
197+
198+
end subroutine test_core_io_test
199+
200+
end module test_core_io

0 commit comments

Comments
 (0)