[update] fix (unexploitable) BB'06 vulnerability in rsa_verify
authorFilippo Valsorda <filippo@cloudflare.com>
Mon, 14 Dec 2015 02:18:13 +0000 (02:18 +0000)
committerFilippo Valsorda <filippo@cloudflare.com>
Thu, 21 Jan 2016 20:12:17 +0000 (20:12 +0000)
The rsa_verify code was vulnerable to a BB'06 attack, allowing to forge
signatures for arbitrary messages if and only if the public key exponent is
3.  Since the updates key is hardcoded to 65537, there is no risk for
youtube-dl, but I don't want vulnerable code in the wild.

The new function adopts a way safer approach of encoding-and-comparing to
replace the dangerous parsing code.

test/test_update.py [new file with mode: 0644]
test/versions.json [new file with mode: 0644]
youtube_dl/update.py

diff --git a/test/test_update.py b/test/test_update.py
new file mode 100644 (file)
index 0000000..d9c7151
--- /dev/null
@@ -0,0 +1,30 @@
+#!/usr/bin/env python
+
+from __future__ import unicode_literals
+
+# Allow direct execution
+import os
+import sys
+import unittest
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+
+import json
+from youtube_dl.update import rsa_verify
+
+
+class TestUpdate(unittest.TestCase):
+    def test_rsa_verify(self):
+        UPDATES_RSA_KEY = (0x9d60ee4d8f805312fdb15a62f87b95bd66177b91df176765d13514a0f1754bcd2057295c5b6f1d35daa6742c3ffc9a82d3e118861c207995a8031e151d863c9927e304576bc80692bc8e094896fcf11b66f3e29e04e3a71e9a11558558acea1840aec37fc396fb6b65dc81a1c4144e03bd1c011de62e3f1357b327d08426fe93, 65537)
+        with open(os.path.join(os.path.dirname(os.path.abspath(__file__)), 'versions.json'), 'rb') as f:
+            versions_info = f.read().decode()
+        versions_info = json.loads(versions_info)
+        signature = versions_info['signature']
+        del versions_info['signature']
+        self.assertTrue(rsa_verify(
+            json.dumps(versions_info, sort_keys=True).encode('utf-8'),
+            signature, UPDATES_RSA_KEY))
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/test/versions.json b/test/versions.json
new file mode 100644 (file)
index 0000000..6cccc22
--- /dev/null
@@ -0,0 +1,34 @@
+{
+    "latest": "2013.01.06", 
+    "signature": "72158cdba391628569ffdbea259afbcf279bbe3d8aeb7492690735dc1cfa6afa754f55c61196f3871d429599ab22f2667f1fec98865527b32632e7f4b3675a7ef0f0fbe084d359256ae4bba68f0d33854e531a70754712f244be71d4b92e664302aa99653ee4df19800d955b6c4149cd2b3f24288d6e4b40b16126e01f4c8ce6", 
+    "versions": {
+        "2013.01.02": {
+            "bin": [
+                "http://youtube-dl.org/downloads/2013.01.02/youtube-dl", 
+                "f5b502f8aaa77675c4884938b1e4871ebca2611813a0c0e74f60c0fbd6dcca6b"
+            ], 
+            "exe": [
+                "http://youtube-dl.org/downloads/2013.01.02/youtube-dl.exe", 
+                "75fa89d2ce297d102ff27675aa9d92545bbc91013f52ec52868c069f4f9f0422"
+            ], 
+            "tar": [
+                "http://youtube-dl.org/downloads/2013.01.02/youtube-dl-2013.01.02.tar.gz", 
+                "6a66d022ac8e1c13da284036288a133ec8dba003b7bd3a5179d0c0daca8c8196"
+            ]
+        }, 
+        "2013.01.06": {
+            "bin": [
+                "http://youtube-dl.org/downloads/2013.01.06/youtube-dl", 
+                "64b6ed8865735c6302e836d4d832577321b4519aa02640dc508580c1ee824049"
+            ], 
+            "exe": [
+                "http://youtube-dl.org/downloads/2013.01.06/youtube-dl.exe", 
+                "58609baf91e4389d36e3ba586e21dab882daaaee537e4448b1265392ae86ff84"
+            ], 
+            "tar": [
+                "http://youtube-dl.org/downloads/2013.01.06/youtube-dl-2013.01.06.tar.gz", 
+                "fe77ab20a95d980ed17a659aa67e371fdd4d656d19c4c7950e7b720b0c2f1a86"
+            ]
+        }
+    }
+}
\ No newline at end of file
index 995b8ed962fe1cafa1dd0f97a868b9a902164351..e4a1aaa641b34ed8c6a33e6b5fb725fce6cdba01 100644 (file)
@@ -15,33 +15,17 @@ from .version import __version__
 
 
 def rsa_verify(message, signature, key):
-    from struct import pack
     from hashlib import sha256
-
     assert isinstance(message, bytes)
-    block_size = 0
-    n = key[0]
-    while n:
-        block_size += 1
-        n >>= 8
-    signature = pow(int(signature, 16), key[1], key[0])
-    raw_bytes = []
-    while signature:
-        raw_bytes.insert(0, pack("B", signature & 0xFF))
-        signature >>= 8
-    signature = (block_size - len(raw_bytes)) * b'\x00' + b''.join(raw_bytes)
-    if signature[0:2] != b'\x00\x01':
-        return False
-    signature = signature[2:]
-    if b'\x00' not in signature:
-        return False
-    signature = signature[signature.index(b'\x00') + 1:]
-    if not signature.startswith(b'\x30\x31\x30\x0D\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x01\x05\x00\x04\x20'):
-        return False
-    signature = signature[19:]
-    if signature != sha256(message).digest():
+    byte_size = (len(bin(key[0])) - 2 + 8 - 1) // 8
+    signature = ('%x' % pow(int(signature, 16), key[1], key[0])).encode()
+    signature = (byte_size * 2 - len(signature)) * b'0' + signature
+    asn1 = b'3031300d060960864801650304020105000420'
+    asn1 += sha256(message).hexdigest().encode()
+    if byte_size < len(asn1) // 2 + 11:
         return False
-    return True
+    expected = b'0001' + (byte_size - len(asn1) // 2 - 3) * b'ff' + b'00' + asn1
+    return expected == signature
 
 
 def update_self(to_screen, verbose, opener):