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

Aperta Adatta ai principianti
#435 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
c
Ambito
backend

Direzione di ricerca

Inizia da src/common/modelParams.c, in particolare da readModelParams() in corrispondenza della tokenizzazione e delle chiamate a strtod(); confronta la gestione dei valori mancanti in src/sipnet/frontend.c:83-88. Poi esamina tests/sipnet/test_sipnet_infrastructure/testNonfiniteParams.c per individuare il modello di test esistente. Il lavoro è concluso quando i valori mancanti e le stringhe numeriche malformate producono gli errori specificati e i test coprono entrambi i casi.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.
Lingua principale
C
Stelle
42
Fork
38
Merge medio
3g 21h
PR unite (30g)
10

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di PecanProject/sipnet

Tutte le issue di PecanProject/sipnet

Issue simili

Altre issue su C

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.