]>
Commit | Line | Data |
---|---|---|
e2bd68df MF |
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 | } |