Merge lp:~niedbalski/ubuntu/vivid/squid3/fix-pinger-icmp into lp:ubuntu/vivid/squid3

Proposed by Jorge Niedbalski
Status: Merged
Merge reported by: Iain Lane
Merged at revision: not available
Proposed branch: lp:~niedbalski/ubuntu/vivid/squid3/fix-pinger-icmp
Merge into: lp:ubuntu/vivid/squid3
Diff against target: 282 lines (+262/-0)
3 files modified
debian/changelog (+6/-0)
debian/patches/fix-icmp-pinger-icmp4-6.patch (+255/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~niedbalski/ubuntu/vivid/squid3/fix-pinger-icmp
Reviewer Review Type Date Requested Status
Iain Lane Approve
Review via email: mp+242098@code.launchpad.net

Description of the change

Fixes LP: #1384943

Description: Fix various ICMP handling issues in Squid pinger

* ICMP code type logging display could over-read the registered type
  string arrays.

* Malformed ICMP packets were accepted into processing with undefined
  and potentially nasty results.

Both sets of flaws can result in pinger segmentation fault and halting
the Squid functionality relying on pinger for correct operation.

To post a comment you must log in.
59. By Jorge Niedbalski

[changelog] Fix various ICMP handling issues in Squid pinger.

Revision history for this message
Iain Lane (laney) wrote :

Thanks, uploaded. It'd probably be good to merge the newer upstream version that Debian has.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2014-08-26 13:51:07 +0000
+++ debian/changelog 2014-11-18 17:48:36 +0000
@@ -1,3 +1,9 @@
1squid3 (3.3.8-1ubuntu9) vivid; urgency=medium
2
3 * Fix various ICMP handling issues in Squid pinger.
4
5 -- Jorge Niedbalski <jorge.niedbalski@canonical.com> Tue, 18 Nov 2014 14:47:33 -0300
6
1squid3 (3.3.8-1ubuntu8) utopic; urgency=medium7squid3 (3.3.8-1ubuntu8) utopic; urgency=medium
28
3 * SECURITY UPDATE: Ignore Range headers with unidentifiable byte-range9 * SECURITY UPDATE: Ignore Range headers with unidentifiable byte-range
410
=== added file 'debian/patches/fix-icmp-pinger-icmp4-6.patch'
--- debian/patches/fix-icmp-pinger-icmp4-6.patch 1970-01-01 00:00:00 +0000
+++ debian/patches/fix-icmp-pinger-icmp4-6.patch 2014-11-18 17:48:36 +0000
@@ -0,0 +1,255 @@
1Description: Fix various ICMP handling issues in Squid pinger
2
3* ICMP code type logging display could over-read the registered type
4 string arrays.
5
6* Malformed ICMP packets were accepted into processing with undefined
7 and potentially nasty results.
8
9Both sets of flaws can result in pinger segmentation fault and halting
10the Squid functionality relying on pinger for correct operation.
11
12Author: Jamie Strandboge <jamie@ubuntu.com>
13Origin: upstream, http://bazaar.launchpad.net/~squid/squid/3.2/revision/11830
14Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/squid3/+bug/1384943
15
16--- squid3-3.3.8.orig/src/icmp/Icmp4.cc
17+++ squid3-3.3.8/src/icmp/Icmp4.cc
18@@ -41,26 +41,38 @@
19 #include "IcmpPinger.h"
20 #include "Debug.h"
21
22-const char *icmpPktStr[] = {
23- "Echo Reply",
24- "ICMP 1",
25- "ICMP 2",
26- "Destination Unreachable",
27- "Source Quench",
28- "Redirect",
29- "ICMP 6",
30- "ICMP 7",
31- "Echo",
32- "ICMP 9",
33- "ICMP 10",
34- "Time Exceeded",
35- "Parameter Problem",
36- "Timestamp",
37- "Timestamp Reply",
38- "Info Request",
39- "Info Reply",
40- "Out of Range Type"
41-};
42+static const char *
43+IcmpPacketType(uint8_t v)
44+{
45+ static const char *icmpPktStr[] = {
46+ "Echo Reply",
47+ "ICMP 1",
48+ "ICMP 2",
49+ "Destination Unreachable",
50+ "Source Quench",
51+ "Redirect",
52+ "ICMP 6",
53+ "ICMP 7",
54+ "Echo",
55+ "ICMP 9",
56+ "ICMP 10",
57+ "Time Exceeded",
58+ "Parameter Problem",
59+ "Timestamp",
60+ "Timestamp Reply",
61+ "Info Request",
62+ "Info Reply",
63+ "Out of Range Type"
64+ };
65+
66+ if (v > 17) {
67+ static char buf[50];
68+ snprintf(buf, sizeof(buf), "ICMP %u (invalid)", v);
69+ return buf;
70+ }
71+
72+ return icmpPktStr[v];
73+}
74
75 Icmp4::Icmp4() : Icmp()
76 {
77@@ -187,6 +199,12 @@ Icmp4::Recv(void)
78 from->ai_addr,
79 &from->ai_addrlen);
80
81+ if (n <= 0) {
82+ debugs(42, DBG_CRITICAL, HERE << "Error when calling recvfrom() on ICMP socket.");
83+ Ip::Address::FreeAddrInfo(from);
84+ return;
85+ }
86+
87 preply.from = *from;
88
89 #if GETTIMEOFDAY_NO_TZP
90@@ -243,9 +261,15 @@ Icmp4::Recv(void)
91
92 preply.psize = n - iphdrlen - (sizeof(icmpEchoData) - MAX_PKT4_SZ);
93
94+ if (preply.psize < 0) {
95+ debugs(42, DBG_CRITICAL, HERE << "Malformed ICMP packet.");
96+ Ip::Address::FreeAddrInfo(from);
97+ return;
98+ }
99+
100 control.SendResult(preply, (sizeof(pingerReplyData) - MAX_PKT4_SZ + preply.psize) );
101
102- Log(preply.from, icmp->icmp_type, icmpPktStr[icmp->icmp_type], preply.rtt, preply.hops);
103+ Log(preply.from, icmp->icmp_type, IcmpPacketType(icmp->icmp_type), preply.rtt, preply.hops);
104 preply.from.FreeAddrInfo(from);
105 }
106
107--- squid3-3.3.8.orig/src/icmp/Icmp6.cc
108+++ squid3-3.3.8/src/icmp/Icmp6.cc
109@@ -51,56 +51,62 @@
110 // Icmp6 OP-Codes
111 // see http://www.iana.org/assignments/icmpv6-parameters
112 // NP: LowPktStr is for codes 0-127
113-static const char *icmp6LowPktStr[] = {
114- "ICMP 0", // 0
115- "Destination Unreachable", // 1 - RFC2463
116- "Packet Too Big", // 2 - RFC2463
117- "Time Exceeded", // 3 - RFC2463
118- "Parameter Problem", // 4 - RFC2463
119- "ICMP 5", // 5
120- "ICMP 6", // 6
121- "ICMP 7", // 7
122- "ICMP 8", // 8
123- "ICMP 9", // 9
124- "ICMP 10" // 10
125-};
126-
127-// NP: HighPktStr is for codes 128-255
128-static const char *icmp6HighPktStr[] = {
129- "Echo Request", // 128 - RFC2463
130- "Echo Reply", // 129 - RFC2463
131- "Multicast Listener Query", // 130 - RFC2710
132- "Multicast Listener Report", // 131 - RFC2710
133- "Multicast Listener Done", // 132 - RFC2710
134- "Router Solicitation", // 133 - RFC4861
135- "Router Advertisement", // 134 - RFC4861
136- "Neighbor Solicitation", // 135 - RFC4861
137- "Neighbor Advertisement", // 136 - RFC4861
138- "Redirect Message", // 137 - RFC4861
139- "Router Renumbering", // 138 - Crawford
140- "ICMP Node Information Query", // 139 - RFC4620
141- "ICMP Node Information Response", // 140 - RFC4620
142- "Inverse Neighbor Discovery Solicitation", // 141 - RFC3122
143- "Inverse Neighbor Discovery Advertisement", // 142 - RFC3122
144- "Version 2 Multicast Listener Report", // 143 - RFC3810
145- "Home Agent Address Discovery Request", // 144 - RFC3775
146- "Home Agent Address Discovery Reply", // 145 - RFC3775
147- "Mobile Prefix Solicitation", // 146 - RFC3775
148- "Mobile Prefix Advertisement", // 147 - RFC3775
149- "Certification Path Solicitation", // 148 - RFC3971
150- "Certification Path Advertisement", // 149 - RFC3971
151- "ICMP Experimental (150)", // 150 - RFC4065
152- "Multicast Router Advertisement", // 151 - RFC4286
153- "Multicast Router Solicitation", // 152 - RFC4286
154- "Multicast Router Termination", // 153 - [RFC4286]
155- "ICMP 154",
156- "ICMP 155",
157- "ICMP 156",
158- "ICMP 157",
159- "ICMP 158",
160- "ICMP 159",
161- "ICMP 160"
162-};
163+
164+static const char *
165+IcmpPacketType(uint8_t v)
166+{
167+ // NP: LowPktStr is for codes 0-127
168+ static const char *icmp6LowPktStr[] = {
169+ "ICMPv6 0", // 0
170+ "Destination Unreachable", // 1 - RFC2463
171+ "Packet Too Big", // 2 - RFC2463
172+ "Time Exceeded", // 3 - RFC2463
173+ "Parameter Problem", // 4 - RFC2463
174+ };
175+
176+ // low codes 1-4 registered
177+ if (0 < v && v < 5)
178+ return icmp6LowPktStr[(int)(v&0x7f)];
179+
180+ // NP: HighPktStr is for codes 128-255
181+ static const char *icmp6HighPktStr[] = {
182+ "Echo Request", // 128 - RFC2463
183+ "Echo Reply", // 129 - RFC2463
184+ "Multicast Listener Query", // 130 - RFC2710
185+ "Multicast Listener Report", // 131 - RFC2710
186+ "Multicast Listener Done", // 132 - RFC2710
187+ "Router Solicitation", // 133 - RFC4861
188+ "Router Advertisement", // 134 - RFC4861
189+ "Neighbor Solicitation", // 135 - RFC4861
190+ "Neighbor Advertisement", // 136 - RFC4861
191+ "Redirect Message", // 137 - RFC4861
192+ "Router Renumbering", // 138 - Crawford
193+ "ICMP Node Information Query", // 139 - RFC4620
194+ "ICMP Node Information Response", // 140 - RFC4620
195+ "Inverse Neighbor Discovery Solicitation", // 141 - RFC3122
196+ "Inverse Neighbor Discovery Advertisement", // 142 - RFC3122
197+ "Version 2 Multicast Listener Report", // 143 - RFC3810
198+ "Home Agent Address Discovery Request", // 144 - RFC3775
199+ "Home Agent Address Discovery Reply", // 145 - RFC3775
200+ "Mobile Prefix Solicitation", // 146 - RFC3775
201+ "Mobile Prefix Advertisement", // 147 - RFC3775
202+ "Certification Path Solicitation", // 148 - RFC3971
203+ "Certification Path Advertisement", // 149 - RFC3971
204+ "ICMP Experimental (150)", // 150 - RFC4065
205+ "Multicast Router Advertisement", // 151 - RFC4286
206+ "Multicast Router Solicitation", // 152 - RFC4286
207+ "Multicast Router Termination", // 153 - [RFC4286]
208+ };
209+
210+ // high codes 127-153 registered
211+ if (127 < v && v < 154)
212+ return icmp6HighPktStr[(int)(v&0x7f)];
213+
214+ // give all others a generic display
215+ static char buf[50];
216+ snprintf(buf, sizeof(buf), "ICMPv6 %u", v);
217+ return buf;
218+}
219
220 Icmp6::Icmp6() : Icmp()
221 {
222@@ -236,6 +242,12 @@ Icmp6::Recv(void)
223 from->ai_addr,
224 &from->ai_addrlen);
225
226+ if (n <= 0) {
227+ debugs(42, DBG_CRITICAL, HERE << "Error when calling recvfrom() on ICMPv6 socket.");
228+ Ip::Address::FreeAddrInfo(from);
229+ return;
230+ }
231+
232 preply.from = *from;
233
234 #if GETTIMEOFDAY_NO_TZP
235@@ -291,9 +303,9 @@ Icmp6::Recv(void)
236
237 default:
238 debugs(42, 8, HERE << preply.from << " said: " << icmp6header->icmp6_type << "/" << (int)icmp6header->icmp6_code << " " <<
239- ( icmp6header->icmp6_type&0x80 ? icmp6HighPktStr[(int)(icmp6header->icmp6_type&0x7f)] : icmp6LowPktStr[(int)(icmp6header->icmp6_type&0x7f)] )
240- );
241+ IcmpPacketType(icmp6header->icmp6_type));
242 }
243+
244 preply.from.FreeAddrInfo(from);
245 return;
246 }
247@@ -331,7 +343,7 @@ Icmp6::Recv(void)
248
249 Log(preply.from,
250 icmp6header->icmp6_type,
251- ( icmp6header->icmp6_type&0x80 ? icmp6HighPktStr[(int)(icmp6header->icmp6_type&0x7f)] : icmp6LowPktStr[(int)(icmp6header->icmp6_type&0x7f)] ),
252+ IcmpPacketType(icmp6header->icmp6_type),
253 preply.rtt,
254 preply.hops);
255
0256
=== modified file 'debian/patches/series'
--- debian/patches/series 2014-08-26 13:51:07 +0000
+++ debian/patches/series 2014-11-18 17:48:36 +0000
@@ -7,3 +7,4 @@
7fix-pod2man-config.test.patch7fix-pod2man-config.test.patch
8fix-distribution.patch8fix-distribution.patch
9CVE-2014-3609.patch9CVE-2014-3609.patch
10fix-icmp-pinger-icmp4-6.patch

Subscribers

People subscribed via source and target branches