squid 3.5.28: latest patches (01-02)
[ipfire-2.x.git] / src / patches / squid / 01_Certificate_fields_injection_via_D_in_ERR_SECURE_CONNECT_FAIL_306.patch
1 commit f1657a9decc820f748fa3aff68168d3145258031
2 Author: Christos Tsantilas <christos@chtsanti.net>
3 Date: 2018-10-17 15:14:07 +0000
4
5 Certificate fields injection via %D in ERR_SECURE_CONNECT_FAIL (#306)
6
7 %ssl_subject, %ssl_ca_name, and %ssl_cn values were not properly escaped when %D code was expanded in HTML context of the ERR_SECURE_CONNECT_FAIL template. This bug affects all
8 ERR_SECURE_CONNECT_FAIL page templates containing %D, including the default template.
9
10 Other error pages are not vulnerable because Squid does not populate %D with certificate details in other contexts (yet).
11
12 Thanks to Nikolas Lohmann [eBlocker] for identifying the problem.
13
14 TODO: If those certificate details become needed for ACL checks or other non-HTML purposes, make their HTML-escaping conditional.
15
16 This is a Measurement Factory project.
17
18 diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
19 index b5030e3..314e998 100644
20 --- a/src/ssl/ErrorDetail.cc
21 +++ b/src/ssl/ErrorDetail.cc
22 @@ -8,6 +8,8 @@
23
24 #include "squid.h"
25 #include "errorpage.h"
26 +#include "fatal.h"
27 +#include "html_quote.h"
28 #include "ssl/ErrorDetail.h"
29
30 #include <climits>
31 @@ -432,8 +434,11 @@ const char *Ssl::ErrorDetail::subject() const
32 {
33 if (broken_cert.get()) {
34 static char tmpBuffer[256]; // A temporary buffer
35 - if (X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer)))
36 - return tmpBuffer;
37 + if (X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer))) {
38 + // quote to avoid possible html code injection through
39 + // certificate subject
40 + return html_quote(tmpBuffer);
41 + }
42 }
43 return "[Not available]";
44 }
45 @@ -461,8 +466,11 @@ const char *Ssl::ErrorDetail::cn() const
46 static String tmpStr; ///< A temporary string buffer
47 tmpStr.clean();
48 Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn);
49 - if (tmpStr.size())
50 - return tmpStr.termedBuf();
51 + if (tmpStr.size()) {
52 + // quote to avoid possible html code injection through
53 + // certificate subject
54 + return html_quote(tmpStr.termedBuf());
55 + }
56 }
57 return "[Not available]";
58 }
59 @@ -474,8 +482,11 @@ const char *Ssl::ErrorDetail::ca_name() const
60 {
61 if (broken_cert.get()) {
62 static char tmpBuffer[256]; // A temporary buffer
63 - if (X509_NAME_oneline(X509_get_issuer_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer)))
64 - return tmpBuffer;
65 + if (X509_NAME_oneline(X509_get_issuer_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer))) {
66 + // quote to avoid possible html code injection through
67 + // certificate issuer subject
68 + return html_quote(tmpBuffer);
69 + }
70 }
71 return "[Not available]";
72 }