Validating the data is pretty easy. The only data is the rate and it
is supposed to be a floating point number. Switching to https is easy too.
The attached patch does both.
Is it enough?
On Tue, Jul 03, 2018 at 09:04:14PM +0200, Stephen Kitt wrote:
> Control: forwarded adri...@gnu.org
>
> Hi Adrian,
>
> I thought you’d be interested in this bug report... A straightforward partial
> fix would be to switch to the https URIs, better still would be to add
> certificate validation of some sort, but I think a real fix would involve
> format validation and data sanitization (as Jakub mentions).
>
> Regards,
>
> Stephen
>
>
> On Tue, 3 Jul 2018 18:55:40 +0200, Jakub Wilk <jw...@jwilk.net> wrote:
>
> > Package: units
> > Version: 2.17-1
> > Tags: security
> >
> > units_cur does no sanitization of the data it downloads. Malicious
> > operators of the servers or man-in-the-middle attackers[*] could exploit
> > this to execute arbitrary code.
> >
> > As a proof of concept, I patched units_cur to emulate Yahoo returning
> > malicious data. After updating the data, /var/lib/units/currency.units
> > looks like this:
> >
> > southkoreawon 1|0
> > !set PAGER cowsay${IFS}pwned;exit;
> > # US$
> >
> > And this happens:
> >
> > $ units
> > Currency exchange rates from finance.yahoo.com on 2018-07-03
> > 3048 units, 109 prefixes, 109 nonlinear units
> >
> > You have: help kg
> > _______
> > < pwned >
> > -------
> > \ ^__^
> > \ (oo)\_______
> > (__)\ )\/\
> > ||----w |
> > || ||
> >
> >
> > [*] Conveniently, all the data in downloaded over HTTP, so there's no
> > authentication.
> >
> > --
> > Jakub Wilk
--- /home/adrian/src/units/units_cur 2018-06-03 23:28:46.023000324 -0400
+++ units_cur 2018-07-20 16:52:16.000000000 -0400
@@ -28,8 +28,12 @@
#
#
-version = '4.2'
+version = '4.3'
+# Version 4.3: 20 July 2018
+#
+# Validate rate data from server
+#
# Version 4.2: 18 April 2018
#
# Handle case of empty/malformed entry returned from the server
@@ -275,7 +279,7 @@
verbose = ap.parse_args().verbose
try:
- res = requests.get('http://finance.yahoo.com/webservice/v1/symbols'
+ res = requests.get('https://finance.yahoo.com/webservice/v1/symbols'
'/allcurrencies/quote?format=json')
res.raise_for_status()
webdata = res.json()['list']['resources']
@@ -299,7 +303,12 @@
stderr.write('Got unknown currency with code {}\n'.format(code))
else:
if not currency[code][rate_index]:
- currency[code][rate_index] = '1|{} US$'.format(rate)
+ try:
+ float(rate)
+ currency[code][rate_index] = '1|{} US$'.format(rate)
+ except ValueError:
+ stderr.write('Got invalid rate "{}" for currency "{}"\n'.format(
+ rate, code))
elif verbose:
stderr.write('Got value "{}" for currency "{}" but '
'it is already defined\n'.format(rate, code))
@@ -313,7 +322,7 @@
del currency[code]
try:
- req = requests.get('http://services.packetizer.com/spotprices/?f=json')
+ req = requests.get('https://services.packetizer.com/spotprices/?f=json')
req.raise_for_status()
metals = req.json()
except requests.exceptions.RequestException as e:
@@ -323,7 +332,7 @@
del metals['date']
try:
- req = requests.get('http://services.packetizer.com/btc/?f=json')
+ req = requests.get('https://services.packetizer.com/btc/?f=json')
req.raise_for_status()
bitcoin = req.json()
except requests.exceptions.RequestException as e:
