[BUG] readModelParams: Segfault on missing parameter value and unvalidated strtod silently accepts non-numeric strings

Open Beginner friendly
  • Is something wrong?
#435 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
backend

Research direction

Start with src/common/modelParams.c, especially readModelParams() around the tokenization and strtod() calls; compare the missing-value handling in src/sipnet/frontend.c:83-88. Then inspect tests/sipnet/test_sipnet_infrastructure/testNonfiniteParams.c for the existing test pattern. Done means missing values and malformed numeric strings produce the specified errors, with tests covering both cases.

Written by the indexing model from the issue text.

Description

Summary

In src/common/modelParams.c:readModelParams(), the .param file reader contains two input validation flaws:

  1. Segmentation Fault (SIGSEGV): When a parameter record has a parameter name without a corresponding value, strtok() returns NULL, and passing it into strcpy() causes an immediate segmentation fault (signal 11 / exit code 139).
  2. Silent Acceptance of Invalid Strings: When a parameter value contains non-numeric characters (e.g., plantWoodCInit not_a_number or soilRespQ10 2.2abc), value = strtod(strValue, &errc) parses the string, but errc is never validated. As a result, strtod() returns 0.0 (or 2.2), and SIPNET silently accepts the corrupted parameter value and runs with exit code 0.

Detailed Problem & Root Cause

1. Crash on Missing Parameter Value

In src/common/modelParams.c:170-172:

// tokenize line:
strcpy(pName, strtok(line, SEPARATORS));  // copy first token into pName

strcpy(strValue, strtok(NULL, SEPARATORS));

When a line in the .param file contains only a parameter name without a following value (for example, plantWoodCInit alone on a line, or trailing whitespace without a value token):

  • strtok(line, SEPARATORS) returns the parameter name.
  • strtok(NULL, SEPARATORS) returns NULL.
  • strcpy(strValue, NULL) invokes undefined behavior and immediately causes a Segmentation fault (core dumped).

In contrast, src/sipnet/frontend.c:83-88 explicitly checks:

if (inputValue == NULL) {
  logError("Error in input file: No value given for input item %s\n", inputName);
  logError("Please fix %s and re-run\n", ctx.inputFile);
  exit(EXIT_CODE_BAD_PARAMETER_VALUE);
}
2. Silent Acceptance of Non-Numeric Parameter Values

In src/common/modelParams.c:149, 180:

char *errc;
...
value = strtod(strValue, &errc);

While errc is passed to strtod(), it is never checked afterwards.
According to the C standard (strtod(3)), if no conversion can be performed, strtod returns 0.0 and sets *errc = strValue. If conversion stops partway through (e.g., 12.5xyz), *errc != '\0'.
Because errc is completely ignored, invalid strings are silently converted to 0.0 (or truncated values), and the model proceeds without any warning or error, silently compromising scientific simulation results.

Steps to Reproduce

Reproduction 1: Segmentation Fault
  1. Add an empty parameter line to sipnet.param: 
    echo "plantWoodCInit" >> tests/smoke/niwot/sipnet.param
  2. Run SIPNET: 
    ./sipnet -i tests/smoke/niwot/sipnet.in
  3. Process crashes with SIGSEGV (exit code 139): 
    Program received signal SIGSEGV, Segmentation fault.
__strcpy_avx2 () at ../sysdeps/x86_64/multiarch/strcpy-avx2.S:80
#0  __strcpy_avx2 ()
#1  0x0000555555575bcb in readModelParams (modelParams=0x555555586d60, paramFile=0x555555586b80) at src/common/modelParams.c:171
#2  0x0000555555557970 in readParamData (modelParamsPtr=0x7fffffffd2c8, paramFile=0x7fffffffd510 "sipnet.param") at src/sipnet/sipnet.c:403
#3  0x000055555555aff4 in initModel (modelParams=0x7fffffffd2c8, paramFile=0x7fffffffd510 "sipnet.param", climFile=0x7fffffffd610 "sipnet.clim") at src/sipnet/sipnet.c:1993
#4  0x00005555555655ca in main (argc=1, argv=0x7fffffffda58) at src/sipnet/frontend.c:212
Reproduction 2: Silent Conversion of Non-Numeric String
  1. Edit any parameter in sipnet.param to a non-numeric string: 
    sed -i 's/^plantWoodCInit .*/plantWoodCInit not_a_number/' sipnet.param
  2. Run SIPNET: 
    ./sipnet
  3. Outcome: SIPNET completes with Exit code: 0, silently treating plantWoodCInit as 0.0.

Proposed Solution

  1. Guard strtok return value:
    Check if the value token is NULL before copying:

    char *pNameToken = strtok(line, SEPARATORS);
char *valToken = strtok(NULL, SEPARATORS);

if (pNameToken == NULL) {
  continue; // empty line / comment
}

if (valToken == NULL) {
  logError("Error in parameter file: No value given for parameter %s\n", pNameToken);
  exit(EXIT_CODE_INPUT_FILE_ERROR);
}

strncpy(pName, pNameToken, sizeof(pName) - 1);
pName[sizeof(pName) - 1] = '\0';
strncpy(strValue, valToken, sizeof(strValue) - 1);
strValue[sizeof(strValue) - 1] = '\0';

  2. Validate errc from strtod:
    Verify that the entire value string was consumed as a valid number:

    value = strtod(strValue, &errc);
if (errc == strValue || *errc != '\0') {
  logError("parameter %s has invalid numeric value: '%s'\n", pName, strValue);
  exit(EXIT_CODE_BAD_PARAMETER_VALUE);
}

  3. Add Unit Tests:
    Extend tests/sipnet/test_sipnet_infrastructure/testNonfiniteParams.c (or add cases to a new unit test suite) verifying that:

    • Parameters with missing values exit with EXIT_CODE_INPUT_FILE_ERROR.
    • Parameters with non-numeric strings ("not_a_number", "12.3foo", etc.) exit with EXIT_CODE_BAD_PARAMETER_VALUE.
Dominant language
C
Stars
42
Forks
38
Avg merge
3d 21h
Merged PRs (30d)
10

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PecanProject/sipnet

All issues in PecanProject/sipnet

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.