1 commit f1657a9decc820f748fa3aff68168d3145258031
2 Author: Christos Tsantilas <christos@chtsanti.net>
3 Date: 2018-10-17 15:14:07 +0000
5 Certificate fields injection via %D in ERR_SECURE_CONNECT_FAIL (#306)
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.
10 Other error pages are not vulnerable because Squid does not populate %D with certificate details in other contexts (yet).
12 Thanks to Nikolas Lohmann [eBlocker] for identifying the problem.
14 TODO: If those certificate details become needed for ACL checks or other non-HTML purposes, make their HTML-escaping conditional.
16 This is a Measurement Factory project.
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
25 #include "errorpage.h"
27 +#include "html_quote.h"
28 #include "ssl/ErrorDetail.h"
31 @@ -432,8 +434,11 @@ const char *Ssl::ErrorDetail::subject() const
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)))
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);
43 return "[Not available]";
45 @@ -461,8 +466,11 @@ const char *Ssl::ErrorDetail::cn() const
46 static String tmpStr; ///< A temporary string buffer
48 Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn);
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());
57 return "[Not available]";
59 @@ -474,8 +482,11 @@ const char *Ssl::ErrorDetail::ca_name() const
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)))
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);
71 return "[Not available]";