Package: apt-cacher-ng
Version: 3-5
Severity: normal
Control: tags -1 + patch
Dear Maintainer,
When generating URLs, apt-cacher-ng (acngtool in particular) does not
properly encode the "userinfo" component (used for credentials in Basic
or Digest HTTP authentication). As a result, when such URLs are later
parsed in order to make HTTP requests, the URL components are
misinterpreted.
I discovered this after investigating why acngtool failed every time
when invoked by cron to perform maintenance. The bug was triggered by
my administration credentials having a "/" character.
Later I noticed that if the credentials contain a "@" character,
acngtool will (partially) leak them in the error message printed upon
failure.
Consider:
- AdminAuth: unexpected.tld:80/secret
- AdminAuth: @dministrator:secret
Below I include a patch for consideration by someone familiar with
the codebase. In my test build it appears to have fixed both issues,
although, I would like to mention that IMHO the URL parsing/encoding/decoding
is still too ad-hoc/klugy and likely to contain bugs, so please review.
Cheers.
PS: Sorry for the admittedly low effort in keeping the code style, I
just couldn't bring myself to do some things.
-- Package-specific info:
-- System Information:
Debian Release: buster/sid
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'),
(500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 4.12.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8),
LANGUAGE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages apt-cacher-ng depends on:
ii adduser 3.116
ii debconf 1.5.63
ii dpkg 1.18.24
ii init-system-helpers 1.49
ii libbz2-1.0 1.0.6-8.1
ii libc6 2.24-17
ii libgcc1 1:7.2.0-7
ii liblzma5 5.2.2-1.3
ii libssl1.1 1.1.0f-5
ii libstdc++6 7.2.0-7
ii libsystemd0 234-3
ii libwrap0 7.6.q-26
ii lsb-base 9.20170808
ii zlib1g 1:1.2.8.dfsg-5
apt-cacher-ng recommends no packages.
Versions of packages apt-cacher-ng suggests:
ii avahi-daemon 0.7-3
pn doc-base <none>
ii libfuse2 2.9.7-1
-- Configuration Files:
/etc/apt-cacher-ng/security.conf [Errno 13] Permission denied:
'/etc/apt-cacher-ng/security.conf'
-- debconf information:
apt-cacher-ng/gentargetmode: No automated setup
apt-cacher-ng/port: keep
apt-cacher-ng/bindaddress: keep
apt-cacher-ng/proxy: keep
apt-cacher-ng/tunnelenable: false
apt-cacher-ng/cachedir: keep
---
--- apt-cacher-ng-3.orig/include/meta.h
+++ apt-cacher-ng-3/include/meta.h
@@ -311,6 +311,8 @@ mstring DosEscape(cmstring &s);
// just the bare minimum to make sure the string does not break HTML formating
mstring html_sanitize(cmstring& in);
+mstring UserinfoEscape(cmstring &s);
+
#define pathTidy(s) { if(startsWithSz(s, "." SZPATHSEP)) s.erase(0, 2);
tStrPos n(0); \
for(n=0;stmiss!=n;) { n=s.find(SZPATHSEP SZPATHSEP, n); if(stmiss!=n)
s.erase(n, 1);}; \
for(n=0;stmiss!=n;) { n=s.find(SZPATHSEP "." SZPATHSEP, n);
if(stmiss!=n) s.erase(n, 2);}; }
--- apt-cacher-ng-3.orig/source/acngtool.cc
+++ apt-cacher-ng-3/source/acngtool.cc
@@ -577,7 +577,7 @@ int maint_job()
tSS urlPath;
urlPath << "http://";;
if (!cfg::adminauth.empty())
- urlPath << cfg::adminauth << "@";
+ urlPath << UserinfoEscape(cfg::adminauth) << "@";
if (hostaddr[0] == '/')
urlPath << "localhost";
else
@@ -1032,7 +1032,7 @@ int wcat(LPCSTR surl, LPCSTR proxy, IFit
if(!surl)
return 2;
string xurl(surl);
- if(!url.SetHttpUrl(xurl))
+ if(!url.SetHttpUrl(xurl, false))
return -2;
dlcon dl(true, nullptr, pDlconFac);
--- apt-cacher-ng-3.orig/source/meta.cc
+++ apt-cacher-ng-3/source/meta.cc
@@ -252,11 +252,11 @@ bool tHttpUrl::SetHttpUrl(cmstring &sUrl
sHost=url.substr(hStart, hEndSuc-hStart);
- // credentials might in there, strip them of
+ // credentials might be in there, strip them off
l=sHost.rfind('@');
if(l!=mstring::npos)
{
- sUserPass=sHost.substr(0, l);
+ sUserPass = UrlUnescape(sHost.substr(0, l));
sHost.erase(0, l+1);
}
@@ -275,16 +275,22 @@ bool tHttpUrl::SetHttpUrl(cmstring &sUrl
strip_ipv6_junk:
+ bool host_appears_to_be_ipv6 = false;
if(sHost[0]=='[')
{
+ host_appears_to_be_ipv6 = true;
bCheckBrac=true;
sHost.erase(0,1);
}
- if(sHost[sHost.length()-1]==']')
+ if (sHost[sHost.length()-1] == ']') {
+ bCheckBrac = !bCheckBrac;
sHost.erase(sHost.length()-1);
- else if(bCheckBrac) // must have been present here
+ }
+ if (bCheckBrac) // Unmatched square brackets.
return false;
+ if (!host_appears_to_be_ipv6)
+ sHost = UrlUnescape(sHost);
return true;
@@ -579,6 +579,63 @@ mstring UrlEscape(cmstring &s)
return ret;
}
+/* From RFC3986 [0]:
+ *
+ * sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
+ * / "*" / "+" / "," / ";" / "="
+ *
+ * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
+ *
+ * authority = [ userinfo "@" ] host [ ":" port ]
+ *
+ * userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
+ *
+ * 0: https://www.ietf.org/rfc/rfc3986.txt
+ */
+static bool is_allowed_unencoded_userinfo_char(char c)
+{
+ switch (c) {
+ // unreserved:
+ case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':
+ case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':
+ case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':
+ case 'V': case 'W': case 'X': case 'Y': case 'Z':
+ case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':
+ case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':
+ case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':
+ case 'v': case 'w': case 'x': case 'y': case 'z':
+ case '0': case '1': case '2': case '3': case '4':
+ case '5': case '6': case '7': case '8': case '9':
+ case '-': case '.': case '_': case '~':
+ // sub-delims:
+ case '!': case '$': case '&': case '\'': case '(': case ')':
+ case '*': case '+': case ',': case ';': case '=':
+ // colon:
+ case ':':
+ return true;
+ default:
+ return false;
+ }
+}
+
+mstring UserinfoEscape(cmstring &s)
+{
+ mstring ret;
+ ret.reserve(s.size());
+
+ for (const auto& c : s) {
+ if (is_allowed_unencoded_userinfo_char(c)) {
+ ret += c;
+ } else {
+ char pct_encoded[4] = { '%', h2t_map[uint8_t(c) >> 4],
+ h2t_map[uint8_t(c) &
0x0f], '\0'};
+ ret += pct_encoded;
+ }
+ }
+
+ return ret;
+}
+
mstring DosEscape(cmstring &s)
{
mstring ret;
