squid 3.5.28: latest patches (01-02)
[people/pmueller/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  }