Fix TOCTOU defect in test_ConfigSavePID()

- Scenarios 1 and 5 fopen() once and fstat() that handle instead of
  stat()ing the path a second time.
- A failed open is now a logged error rather than an indirect rd == 0.
- The FIFO lstat() and failPath existence test stay as they are: neither
  has a descriptor to stat, and mkdtemp()'s 0700 directory makes them
  unraceable.

Issues: CID-651701
pull/1116/head
John Safranek 2026-07-29 09:00:17 -07:00 committed by philljj
parent fc099bc974
commit c39d555d09
1 changed files with 20 additions and 12 deletions

View File

@ -2824,6 +2824,9 @@ static int test_ConfigSavePID(void)
void (*prevXfsz)(int);
#endif
/* Every path below lives in this directory, which mkdtemp() creates mode
* 0700. No other user can traverse it, so none of the path-based checks
* here can be raced. */
if (mkdtemp(base) == NULL) {
Log(" mkdtemp failed.\n");
ret = WS_FATAL_ERROR;
@ -2850,15 +2853,16 @@ static int test_ConfigSavePID(void)
ret = pidSave(conf, pidPath);
}
if (ret == WS_SUCCESS) {
if (stat(pidPath, &st) != 0) {
/* fstat() the open handle instead of stat()ing the path a second
* time, so the mode asserted below belongs to the same file the PID
* was read from. */
f = fopen(pidPath, "r");
if (f == NULL || fstat(fileno(f), &st) != 0) {
Log(" Could not open or stat %s.\n", pidPath);
ret = WS_FATAL_ERROR;
}
else {
f = fopen(pidPath, "r");
rd = (f != NULL) ? fscanf(f, "%ld", &pid) : 0;
if (f != NULL) {
fclose(f);
}
rd = fscanf(f, "%ld", &pid);
ret = smExpect("normal PID file written with our PID",
(rd == 1 && pid == (long)getpid()) ? WS_SUCCESS
: WS_FATAL_ERROR, 1);
@ -2868,6 +2872,9 @@ static int test_ConfigSavePID(void)
: WS_SUCCESS, 1);
}
}
if (f != NULL) {
fclose(f);
}
}
/* Scenario 2: a symlink at the PID path is refused, link target untouched.
@ -2974,15 +2981,13 @@ static int test_ConfigSavePID(void)
}
if (ret == WS_SUCCESS) {
pid = -1;
if (stat(stale, &st) != 0) {
f = fopen(stale, "r");
if (f == NULL || fstat(fileno(f), &st) != 0) {
Log(" Could not open or stat %s.\n", stale);
ret = WS_FATAL_ERROR;
}
else {
f = fopen(stale, "r");
rd = (f != NULL) ? fscanf(f, "%ld", &pid) : 0;
if (f != NULL) {
fclose(f);
}
rd = fscanf(f, "%ld", &pid);
ret = smExpect("existing PID file rewritten with our PID",
(rd == 1 && pid == (long)getpid()) ? WS_SUCCESS
: WS_FATAL_ERROR, 1);
@ -2993,6 +2998,9 @@ static int test_ConfigSavePID(void)
: WS_SUCCESS, 1);
}
}
if (f != NULL) {
fclose(f);
}
}
/* Scenario 6: a PID file owned by another user is refused instead of being