Merge branch '370-confidential-issue-ssrf' into 'develop'

Resolve "SSRF in file upload file URL"

Closes #370

See merge request bramw/baserow!198

backend/requirements/base.txt

@@ -17,3 +17,4 @@ Pillow==8.0.1
 channels==3.0.3
 channels-redis==3.2.0
 celery[redis]==5.0.5
+advocate==1.0.0

backend/src/baserow/api/user_files/errors.py

@@ -18,6 +18,11 @@ ERROR_FILE_URL_COULD_NOT_BE_REACHED = (
     HTTP_400_BAD_REQUEST,
     'The provided URL could not be reached.'
 )
+ERROR_INVALID_FILE_URL = (
+    'ERROR_INVALID_FILE_URL',
+    HTTP_400_BAD_REQUEST,
+    'The provided URL is not valid.'
+)
 ERROR_INVALID_USER_FILE_NAME_ERROR = (
     'ERROR_INVALID_USER_FILE_NAME_ERROR',
     HTTP_400_BAD_REQUEST,

backend/src/baserow/api/user_files/views.py

@@ -11,13 +11,15 @@ from rest_framework.permissions import IsAuthenticated
 from baserow.api.decorators import map_exceptions, validate_body
 from baserow.api.schemas import get_error_schema
 from baserow.core.user_files.exceptions import (
-    InvalidFileStreamError, FileSizeTooLargeError, FileURLCouldNotBeReached
+    InvalidFileStreamError, FileSizeTooLargeError, FileURLCouldNotBeReached,
+    InvalidFileURLError
 )
 from baserow.core.user_files.handler import UserFileHandler
 
 from .serializers import UserFileSerializer, UserFileUploadViaURLRequestSerializer
 from .errors import (
-    ERROR_INVALID_FILE, ERROR_FILE_SIZE_TOO_LARGE, ERROR_FILE_URL_COULD_NOT_BE_REACHED
+    ERROR_INVALID_FILE, ERROR_FILE_SIZE_TOO_LARGE, ERROR_FILE_URL_COULD_NOT_BE_REACHED,
+    ERROR_INVALID_FILE_URL
 )
 
 
@@ -70,7 +72,8 @@ class UploadViaURLView(APIView):
             400: get_error_schema([
                 'ERROR_INVALID_FILE',
                 'ERROR_FILE_SIZE_TOO_LARGE',
-                'ERROR_FILE_URL_COULD_NOT_BE_REACHED'
+                'ERROR_FILE_URL_COULD_NOT_BE_REACHED',
+                'ERROR_INVALID_FILE_URL'
             ])
         }
     )
@@ -78,7 +81,8 @@ class UploadViaURLView(APIView):
     @map_exceptions({
         InvalidFileStreamError: ERROR_INVALID_FILE,
         FileSizeTooLargeError: ERROR_FILE_SIZE_TOO_LARGE,
-        FileURLCouldNotBeReached: ERROR_FILE_URL_COULD_NOT_BE_REACHED
+        FileURLCouldNotBeReached: ERROR_FILE_URL_COULD_NOT_BE_REACHED,
+        InvalidFileURLError: ERROR_INVALID_FILE_URL
     })
     @validate_body(UserFileUploadViaURLRequestSerializer)
     def post(self, request, data):

backend/src/baserow/core/user_files/exceptions.py

@@ -17,6 +17,12 @@ class FileURLCouldNotBeReached(Exception):
     """Raised when the provided URL could not be reached."""
 
 
+class InvalidFileURLError(Exception):
+    """
+    Raised when the provided file URL is invalid or points to an internal service.
+    """
+
+
 class InvalidUserFileNameError(Exception):
     """Raised when the provided user file name is invalid."""
 

backend/src/baserow/core/user_files/handler.py

@@ -1,9 +1,12 @@
 import pathlib
 import mimetypes
+
 from os.path import join
 from io import BytesIO
+from urllib.parse import urlparse
 
-import requests
+import advocate
+from advocate.exceptions import UnacceptableAddressException
 from requests.exceptions import RequestException
 
 from PIL import Image, ImageOps
@@ -16,7 +19,7 @@ from baserow.core.utils import sha256_hash, stream_size, random_string, truncate
 
 from .exceptions import (
     InvalidFileStreamError, FileSizeTooLargeError, FileURLCouldNotBeReached,
-    MaximumUniqueTriesError
+    MaximumUniqueTriesError, InvalidFileURLError
 )
 from .models import UserFile
 
@@ -242,14 +245,20 @@ class UserFileHandler:
         :type storage: Storage
         :raises FileURLCouldNotBeReached: If the file could not be downloaded from
             the URL.
+        :raises InvalidFileURLError: If the provided file url is invalid.
         :return: The newly created user file.
         :rtype: UserFile
         """
 
+        parsed_url = urlparse(url)
+
+        if parsed_url.scheme not in ['http', 'https']:
+            raise InvalidFileURLError('Only http and https are allowed.')
+
         file_name = url.split('/')[-1]
 
         try:
-            response = requests.get(url, stream=True, timeout=10)
+            response = advocate.get(url, stream=True, timeout=10)
 
             if not response.ok:
                 raise FileURLCouldNotBeReached('The response did not respond with an '
@@ -259,7 +268,7 @@ class UserFileHandler:
                 settings.USER_FILE_SIZE_LIMIT + 1,
                 decode_content=True
             )
-        except RequestException:
+        except (RequestException, UnacceptableAddressException):
             raise FileURLCouldNotBeReached('The provided URL could not be reached.')
 
         file = SimpleUploadedFile(file_name, content)

backend/tests/baserow/api/user_files/test_user_files_views.py

@@ -167,12 +167,21 @@ def test_upload_file_via_url(api_client, data_fixture, tmpdir):
 
     response = api_client.post(
         reverse('api:user_files:upload_via_url'),
-        data={'url': 'http://localhost/test2.txt'},
+        data={'url': 'https://baserow.io/test2.txt'},
         HTTP_AUTHORIZATION=f'JWT {token}'
     )
     assert response.status_code == HTTP_400_BAD_REQUEST
     assert response.json()['error'] == 'ERROR_FILE_URL_COULD_NOT_BE_REACHED'
 
+    # Only the http and https protocol are allowed.
+    response = api_client.post(
+        reverse('api:user_files:upload_via_url'),
+        data={'url': 'ftp://baserow.io/test2.txt'},
+        HTTP_AUTHORIZATION=f'JWT {token}'
+    )
+    assert response.status_code == HTTP_400_BAD_REQUEST
+    assert response.json()['error'] == 'ERROR_INVALID_FILE_URL'
+
     responses.add(
         responses.GET,
         'http://localhost/test.txt',
@@ -215,3 +224,19 @@ def test_upload_file_via_url(api_client, data_fixture, tmpdir):
     user_file = UserFile.objects.all().last()
     file_path = tmpdir.join('user_files', user_file.name)
     assert file_path.isfile()
+
+
+@pytest.mark.django_db
+def test_upload_file_via_url_within_private_network(api_client, data_fixture, tmpdir):
+    user, token = data_fixture.create_user_and_token(
+        email='test@test.nl', password='password', first_name='Test1'
+    )
+
+    # Could not be reached because it is an internal private URL.
+    response = api_client.post(
+        reverse('api:user_files:upload_via_url'),
+        data={'url': 'https://localhost/test2.txt'},
+        HTTP_AUTHORIZATION=f'JWT {token}'
+    )
+    assert response.status_code == HTTP_400_BAD_REQUEST
+    assert response.json()['error'] == 'ERROR_FILE_URL_COULD_NOT_BE_REACHED'

backend/tests/baserow/core/user_file/test_user_file_handler.py

@@ -1,6 +1,7 @@
 import pytest
 import responses
 import string
+from unittest.mock import patch
 
 from freezegun import freeze_time
 from PIL import Image
@@ -13,7 +14,7 @@ from django.core.files.storage import FileSystemStorage
 from baserow.core.models import UserFile
 from baserow.core.user_files.exceptions import (
     InvalidFileStreamError, FileSizeTooLargeError, FileURLCouldNotBeReached,
-    MaximumUniqueTriesError
+    MaximumUniqueTriesError, InvalidFileURLError
 )
 from baserow.core.user_files.handler import UserFileHandler
 
@@ -263,7 +264,7 @@ def test_upload_user_file_by_url(data_fixture, tmpdir):
 
     responses.add(
         responses.GET,
-        'http://localhost/test.txt',
+        'https://baserow.io/test.txt',
         body=b'Hello World',
         status=200,
         content_type="text/plain",
@@ -272,31 +273,30 @@ def test_upload_user_file_by_url(data_fixture, tmpdir):
 
     responses.add(
         responses.GET,
-        'http://localhost/not-found.pdf',
-        body=b'Hello World',
+        'https://baserow.io/not-found.pdf',
         status=404,
-        content_type="application/pdf",
-        stream=True,
     )
 
+    # Could not be reached because it it responds with a 404
     with pytest.raises(FileURLCouldNotBeReached):
         handler.upload_user_file_by_url(
             user,
-            'http://localhost/test2.txt',
+            'https://baserow.io/not-found.pdf',
+            storage=storage
+        )
+
+    # Only the http and https protocol are supported.
+    with pytest.raises(InvalidFileURLError):
+        handler.upload_user_file_by_url(
+            user,
+            'ftp://baserow.io/not-found.pdf',
             storage=storage
         )
 
     with freeze_time('2020-01-01 12:00'):
         user_file = handler.upload_user_file_by_url(
             user,
-            'http://localhost/test.txt',
-            storage=storage
-        )
-
-    with pytest.raises(FileURLCouldNotBeReached):
-        handler.upload_user_file_by_url(
-            user,
-            'http://localhost/not-found.pdf',
+            'https://baserow.io/test.txt',
             storage=storage
         )
 
@@ -318,3 +318,26 @@ def test_upload_user_file_by_url(data_fixture, tmpdir):
     file_path = tmpdir.join('user_files', user_file.name)
     assert file_path.isfile()
     assert file_path.open().read() == 'Hello World'
+
+
+@pytest.mark.django_db
+def test_upload_user_file_by_url_within_private_network(data_fixture, tmpdir):
+    user = data_fixture.create_user()
+
+    storage = FileSystemStorage(location=str(tmpdir), base_url='http://localhost')
+    handler = UserFileHandler()
+
+    # Could not be reached because it is an internal private URL.
+    with pytest.raises(FileURLCouldNotBeReached):
+        handler.upload_user_file_by_url(
+            user,
+            'http://localhost/test.txt',
+            storage=storage
+        )
+
+    with pytest.raises(FileURLCouldNotBeReached):
+        handler.upload_user_file_by_url(
+            user,
+            'http://192.168.1.1/test.txt',
+            storage=storage
+        )

changelog.md

@@ -17,7 +17,7 @@
 * Show an error to the user when the web socket connection could not be made and the
   reconnect loop stops.
 * Fixed 100X backend web socket errors when refreshing the page.
-* Prevented the date field value being negative.
+* Fixed SSRF bug in the file upload by URL by blocking urls to the private network.
 
 ## Released (2021-03-01)
 

web-frontend/modules/core/plugins/clientHandler.js

@@ -52,6 +52,10 @@ class ErrorHandler {
         'Invalid URL',
         'The provided file URL could not be reached.'
       ),
+      ERROR_INVALID_FILE_URL: new ResponseErrorMessage(
+        'Invalid URL',
+        'The provided file URL is invalid or not allowed.'
+      ),
     }
 
     // A temporary notFoundMap containing the error messages for when the