1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
|
From 6014c6648a2a54a4ecb7f952ea1163e0798f9264 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 27 Oct 2023 21:27:20 +0000
Subject: [PATCH] Exit without asserting when helper process startup fails
(#1543)
... to dup() after fork() and before execvp().
Assertions are for handling program logic errors. Helper initialization
code already handled system call errors correctly (i.e. by exiting the
newly created helper process with an error), except for a couple of
assert()s that could be triggered by dup(2) failures.
This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ipc-assert.html
where it was filed as 'Assertion in Squid "Helper" Process Creator'.
Origin: http://www.squid-cache.org/Versions/v6/SQUID-2023_8.patch
Upstream-Status: Backport [https://github.com/squid-cache/squid/commit/6014c6648a2a54a4ecb7f952ea1163e0798f9264]
CVE: CVE-2023-49286
Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
---
src/ipc.cc | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
--- a/src/ipc.cc
+++ b/src/ipc.cc
@@ -20,6 +20,12 @@
#include "SquidIpc.h"
#include "tools.h"
+#include <cstdlib>
+
+#if HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
static const char *hello_string = "hi there\n";
#ifndef HELLO_BUF_SZ
#define HELLO_BUF_SZ 32
@@ -365,6 +371,22 @@
}
PutEnvironment();
+
+ // A dup(2) wrapper that reports and exits the process on errors. The
+ // exiting logic is only suitable for this child process context.
+ const auto dupOrExit = [prog,name](const int oldFd) {
+ const auto newFd = dup(oldFd);
+ if (newFd < 0) {
+ const auto savedErrno = errno;
+ debugs(54, DBG_CRITICAL, "ERROR: Helper process initialization failure: " << name);
+ debugs(54, DBG_CRITICAL, "helper (CHILD) PID: " << getpid());
+ debugs(54, DBG_CRITICAL, "helper program name: " << prog);
+ debugs(54, DBG_CRITICAL, "dup(2) system call error for FD " << oldFd << ": " << xstrerr(savedErrno));
+ _exit(1);
+ }
+ return newFd;
+ };
+
/*
* This double-dup stuff avoids problems when one of
* crfd, cwfd, or debug_log are in the rage 0-2.
@@ -372,17 +394,16 @@
do {
/* First make sure 0-2 is occupied by something. Gets cleaned up later */
- x = dup(crfd);
- assert(x > -1);
- } while (x < 3 && x > -1);
+ x = dupOrExit(crfd);
+ } while (x < 3);
close(x);
- t1 = dup(crfd);
+ t1 = dupOrExit(crfd);
- t2 = dup(cwfd);
+ t2 = dupOrExit(cwfd);
- t3 = dup(fileno(debug_log));
+ t3 = dupOrExit(fileno(debug_log));
assert(t1 > 2 && t2 > 2 && t3 > 2);
|
