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
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
|
From: Patrice Fournier <patrice.fournier@ifax.com>
Date: Tue, 18 Sep 2018 03:00:53 +0000 (-0400)
Subject: Address CVE-2018-17141 and fixes a few vulnerabilities in code supporting JPEG
X-Git-Tag: HYLAFAX-6_0_7~1
X-Git-Url: http://git.hylafax.org/HylaFAX?a=commitdiff_plain;h=82fa7bdbffc253de4d3e80a87d47fdbf68eabe36;hp=5b95b384dd1b44b9d2c5c15cc10e50def7c1555d
Address CVE-2018-17141 and fixes a few vulnerabilities in code supporting JPEG
These changes are adapted from Lee's fix for this vulnerability.
Luis Merino, Markus Vervier, and Eric Sesterhenn of X41 D-SEC GmbH
(Security Advisory: X41-2018-008) discovered an uninitialized pointer write
and also an out-of-bounds write in FaxModem::writeECMData() that could lead
to remote code execution with a specially-crafted fax sender.
These changes fix the coding errors and deliberately prevent malicious and
malfunctioning senders from inadvertently or deliberately setting JPEG and
MH/MR/MMR/JBIG formats in the same DCS signal.
---
diff --git a/faxd/Class2.c++ b/faxd/Class2.c++
index 9bd312d..6439719 100644
--- a/faxd/Class2.c++
+++ b/faxd/Class2.c++
@@ -485,6 +485,15 @@ Class2Modem::parseClass2Capabilities(const char* cap, Class2Params& params, bool
} else {
if (jpscan == 0x1) params.jp = JP_GREY;
else if (jpscan & 0x2) params.jp = JP_COLOR;
+ /*
+ * ITU T.30 does not specify that bits 16 (MR) or 31 (MMR) must be set to zero if color fax is used;
+ * and ITU T.32 Table 21 provides a data field, "JP", for JPEG support separate from "DF" for data
+ * format and does not specify that DF is meaningless in DCS when JP is used; but because T.4/T.6
+ * (MH/MR/MMR), JBIG, and JPEG are distinct formats from each other, we must conclude that any
+ * indication of JPEG in DCS must, therefore, invalidate any indication in DCS of MH/MR/MMR/JBIG.
+ * Otherwise, having both df and jp be non-zero will be confusing and possibly cause problems.
+ */
+ if (params.jp != JP_NONE) params.df = 0; // Yes, this is DF_1DMH, but there is no "DF_NONE".
}
return (true);
} else {
diff --git a/faxd/CopyQuality.c++ b/faxd/CopyQuality.c++
index 6ebc936..d1f2d0f 100644
--- a/faxd/CopyQuality.c++
+++ b/faxd/CopyQuality.c++
@@ -38,6 +38,7 @@
#include <ctype.h>
#define RCVBUFSIZ (32*1024) // XXX
+#define COLORBUFSIZ (2000*1024) // 1MB is not big enough
static void setupCompression(TIFF*, u_int, u_int, uint32);
@@ -356,7 +357,7 @@ FaxModem::recvPageDLEData(TIFF* tif, bool checkQuality,
* rather fax-specific.
*/
recvEOLCount = 0;
- recvRow = (u_char*) malloc(1024*1000); // 1M should do it?
+ recvRow = (u_char*) malloc(COLORBUFSIZ);
fxAssert(recvRow != NULL, "page buffering error (JPEG page).");
recvPageStart = recvRow;
}
@@ -408,8 +409,12 @@ FaxModem::recvPageDLEData(TIFF* tif, bool checkQuality,
if (params.df == DF_JBIG) {
flushRawData(tif, 0, (const u_char*) buf, cc);
} else {
- memcpy(recvRow, (const char*) buf, cc);
- recvRow += cc;
+ /* We don't support reception of a JPEG page bigger than COLORBUFSIZ. */
+ if (recvRow + cc - recvPageStart > COLORBUFSIZ) cc = recvPageStart + COLORBUFSIZ - recvRow;
+ if (cc > 0) {
+ memcpy(recvRow, (const char*) buf, cc);
+ recvRow += cc;
+ }
}
} while (!fin);
if (params.df == DF_JBIG) clearSDNORMCount();
@@ -987,7 +992,7 @@ FaxModem::writeECMData(TIFF* tif, u_char* buf, u_int cc, const Class2Params& par
case JP_GREY+4:
case JP_COLOR+4:
recvEOLCount = 0;
- recvRow = (u_char*) malloc(1024*1000); // 1M should do it?
+ recvRow = (u_char*) malloc(COLORBUFSIZ);
fxAssert(recvRow != NULL, "page buffering error (JPEG page).");
recvPageStart = recvRow;
setupStartPage(tif, params);
@@ -1039,14 +1044,20 @@ FaxModem::writeECMData(TIFF* tif, u_char* buf, u_int cc, const Class2Params& par
}
break;
}
- if (params.jp != JP_GREY && params.jp != JP_COLOR) {
- flushRawData(tif, 0, (const u_char*) buf, cc);
- } else {
- memcpy(recvRow, (const char*) buf, cc);
- recvRow += cc;
- }
- if (seq & 2 && (params.jp == JP_GREY || params.jp == JP_COLOR)) {
- fixupJPEG(tif);
+ switch (dataform) {
+ case JP_GREY+4:
+ case JP_COLOR+4:
+ /* We don't support reception of a JPEG page bigger than COLORBUFSIZ. */
+ if (recvRow + cc - recvPageStart > COLORBUFSIZ) cc = recvPageStart + COLORBUFSIZ - recvRow;
+ if (cc > 0) {
+ memcpy(recvRow, (const char*) buf, cc);
+ recvRow += cc;
+ }
+ if (seq & 2) fixupJPEG(tif);
+ break;
+ default:
+ flushRawData(tif, 0, (const u_char*) buf, cc);
+ break;
}
}
diff --git a/libhylafax/Class2Params.c++ b/libhylafax/Class2Params.c++
index 0409cbd..81b9a22 100644
--- a/libhylafax/Class2Params.c++
+++ b/libhylafax/Class2Params.c++
@@ -303,6 +303,15 @@ Class2Params::setFromDCS(FaxParams& dcs_caps)
if (dcs_caps.isBitEnabled(FaxParams::BITNUM_FULLCOLOR)) {
if (jp == JP_GREY) jp = JP_COLOR;
}
+ /*
+ * ITU T.30 does not specify that bits 16 (MR) or 31 (MMR) must be set to zero if color fax is used;
+ * and ITU T.32 Table 21 provides a data field, "JP", for JPEG support separate from "DF" for data
+ * format and does not specify that DF is meaningless in DCS when JP is used; but because T.4/T.6
+ * (MH/MR/MMR), JBIG, and JPEG are distinct formats from each other, we must conclude that any
+ * indication of JPEG in DCS must, therefore, invalidate any indication in DCS of MH/MR/MMR/JBIG.
+ * Otherwise, having both df and jp be non-zero will be confusing and possibly cause problems.
+ */
+ if (jp != JP_NONE) df = 0; // Yes, this is DF_1DMH, but there is no "DF_NONE".
if (ec == EC_DISABLE &&
(df == DF_2DMMR || df == DF_JBIG || jp == JP_GREY || jp == JP_COLOR)) {
// MMR, JBIG, and JPEG require ECM... we've seen cases where fax
|
