Merge pull request #7776 from RasmusWL/django-filefield-uploadto

Python: Support Django FileField.upload_to

Author: yoff

python/ql/lib/semmle/python/frameworks/Django.qll

@@ -737,6 +737,38 @@ module PrivateDjango {
           }
         }
 
+        /**
+         * Provides models for the `django.db.models.FileField` class and `ImageField` subclasses.
+         *
+         * See
+         * - https://docs.djangoproject.com/en/3.1/ref/models/fields/#django.db.models.FileField
+         * - https://docs.djangoproject.com/en/3.1/ref/models/fields/#django.db.models.ImageField
+         */
+        module FileField {
+          /** Gets a reference to the `django.db.models.FileField` or  the `django.db.models.ImageField` class or any subclass. */
+          API::Node subclassRef() {
+            exists(string className | className in ["FileField", "ImageField"] |
+              // commonly used alias
+              result =
+                API::moduleImport("django")
+                    .getMember("db")
+                    .getMember("models")
+                    .getMember(className)
+                    .getASubclass*()
+              or
+              // actual class definition
+              result =
+                API::moduleImport("django")
+                    .getMember("db")
+                    .getMember("models")
+                    .getMember("fields")
+                    .getMember("files")
+                    .getMember(className)
+                    .getASubclass*()
+            )
+          }
+        }
+
         /**
          * Gets a reference to the Manager (django.db.models.Manager) for the django Model `modelClass`,
          * accessed by `<modelClass>.objects`.
@@ -2599,6 +2631,36 @@ module PrivateDjango {
     }
   }
 
+  /**
+   * A parameter that accepts the filename used to upload a file. This is the second
+   * parameter in functions used for the `upload_to` argument to a `FileField`.
+   *
+   * Note that the value this parameter accepts cannot contain a slash. Even when
+   * forcing the filename to contain a slash when sending the request, django does
+   * something like `input_filename.split("/")[-1]` (so other special characters still
+   * allowed). This also means that although the return value from `upload_to` is used
+   * to construct a path, path injection is not possible.
+   *
+   * See
+   *  - https://docs.djangoproject.com/en/3.1/ref/models/fields/#django.db.models.FileField.upload_to
+   *  - https://docs.djangoproject.com/en/3.1/topics/http/file-uploads/#handling-uploaded-files-with-a-model
+   */
+  private class DjangoFileFieldUploadToFunctionFilenameParam extends RemoteFlowSource::Range,
+    DataFlow::ParameterNode {
+    DjangoFileFieldUploadToFunctionFilenameParam() {
+      exists(DataFlow::CallCfgNode call, DataFlow::Node uploadToArg, Function func |
+        this.getParameter() = func.getArg(1) and
+        call = DjangoImpl::DB::Models::FileField::subclassRef().getACall() and
+        uploadToArg in [call.getArg(2), call.getArgByName("upload_to")] and
+        uploadToArg = poorMansFunctionTracker(func)
+      )
+    }
+
+    override string getSourceType() {
+      result = "django filename parameter to function used in FileField.upload_to"
+    }
+  }
+
   // ---------------------------------------------------------------------------
   // django.shortcuts.redirect
   // ---------------------------------------------------------------------------

python/ql/test/library-tests/frameworks/django-v2-v3/.gitignore

@@ -0,0 +1,5 @@
+db.sqlite3
+
+# The testapp/migrations/ folder needs to be comitted to git,
+# but we don't care to store the actual migrations
+testapp/migrations/

python/ql/test/library-tests/frameworks/django-v2-v3/FileField_test.py

@@ -0,0 +1,27 @@
+from django.db import models
+import django.db.models.fields.files
+
+def custom_path_function_1(instance, filename):
+    ensure_tainted(filename) # $ tainted
+
+def custom_path_function_2(instance, filename):
+    ensure_tainted(filename) # $ tainted
+
+def custom_path_function_3(instance, filename):
+    ensure_tainted(filename) # $ tainted
+
+def custom_path_function_4(instance, filename):
+    ensure_tainted(filename) # $ tainted
+
+
+class CustomFileFieldSubclass(models.FileField):
+    pass
+
+
+class MyModel(models.Model):
+    upload_1 = models.FileField(None, None, custom_path_function_1)
+    upload_2 = django.db.models.fields.files.FileField(upload_to=custom_path_function_2)
+
+    upload_3 = models.ImageField(upload_to=custom_path_function_3)
+
+    upload_4 = CustomFileFieldSubclass(upload_to=custom_path_function_4)

python/ql/test/library-tests/frameworks/django-v2-v3/test_file_field

@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+
+# first run the server with
+# python manage.py makemigrations && python manage.py migrate && python manage.py runserver
+
+import requests
+
+requests.post(
+    "http://127.0.0.1:8000/app/file-test/",
+    files={"fieldname": ("foo/bar", open("/home/rasmus/TODO", "rb"))}
+)
+
+requests.post(
+    "http://127.0.0.1:8000/app/file-test/",
+    files={"fieldname": ("../bar", open("/home/rasmus/TODO", "rb"))}
+)
+
+requests.post(
+    "http://127.0.0.1:8000/app/file-test/",
+    files={"fieldname": (r"foo%2fbar", open("/home/rasmus/TODO", "rb"))}
+)
+
+requests.post(
+    "http://127.0.0.1:8000/app/file-test/",
+    files={"fieldname": (r"%2e%2e%2fbar", open("/home/rasmus/TODO", "rb"))}
+)
+
+requests.post(
+    "http://127.0.0.1:8000/app/file-test/",
+    files={"fieldname": (r"foo%c0%afbar", open("/home/rasmus/TODO", "rb"))}
+)

python/ql/test/library-tests/frameworks/django-v2-v3/testapp/models.py

@@ -1,3 +1,10 @@
+import os.path
+
 from django.db import models
 
-# Create your models here.
+def custom_path_function(instance, filename):
+    print(repr(os.path.join("rootdir", filename)))
+    raise NotImplementedError()
+
+class MyModel(models.Model):
+    upload = models.FileField(upload_to=custom_path_function)

python/ql/test/library-tests/frameworks/django-v2-v3/testapp/urls.py

@@ -1,8 +1,5 @@
 from django.urls import path, re_path
 
-# This version 1.x way of defining urls is deprecated in Django 3.1, but still works
-from django.conf.urls import url
-
 from . import views
 
 urlpatterns = [
@@ -11,11 +8,29 @@
     # inline expectation tests (which thinks the `$` would mark the beginning of a new
     # line)
     re_path(r"^ba[rz]/", views.bar_baz),  # $routeSetup="^ba[rz]/"
-    url(r"^deprecated/", views.deprecated),  # $routeSetup="^deprecated/"
 
     path("basic-view-handler/", views.MyBasicViewHandler.as_view()),  # $routeSetup="basic-view-handler/"
     path("custom-inheritance-view-handler/", views.MyViewHandlerWithCustomInheritance.as_view()),  # $routeSetup="custom-inheritance-view-handler/"
 
     path("CustomRedirectView/<foo>", views.CustomRedirectView.as_view()),  # $routeSetup="CustomRedirectView/<foo>"
     path("CustomRedirectView2/<foo>", views.CustomRedirectView2.as_view()),  # $routeSetup="CustomRedirectView2/<foo>"
+
+    path("file-test/", views.file_test), # $routeSetup="file-test/"
 ]
+
+from django import __version__ as django_version
+
+if django_version[0] == "3":
+    # This version 1.x way of defining urls is deprecated in Django 3.1, but still works.
+    # However, it is removed in Django 4.0, so we need this guard to make our code runnable
+    from django.conf.urls import url
+
+    old_urlpatterns = urlpatterns
+
+    # we need this assignment to get our logic working... maybe it should be more
+    # sophisticated?
+    urlpatterns = [
+        url(r"^deprecated/", views.deprecated),  # $routeSetup="^deprecated/"
+    ]
+
+    urlpatterns += old_urlpatterns

python/ql/test/library-tests/frameworks/django-v2-v3/testapp/views.py

@@ -2,6 +2,7 @@
 from django.views.generic import View, RedirectView
 from django.views.decorators.csrf import csrf_exempt
 
+from .models import MyModel
 
 def foo(request: HttpRequest):  # $requestHandler
     return HttpResponse("foo")  # $HttpResponse
@@ -45,3 +46,13 @@ def get_redirect_url(self, foo): # $ requestHandler routedParameter=foo
 class CustomRedirectView2(RedirectView):
 
     url = "https://example.com/%(foo)s"
+
+
+# Test of FileField upload_to functions
+def file_test(request: HttpRequest): # $ requestHandler
+    model = MyModel(upload=request.FILES['fieldname'])
+    try:
+        model.save()
+    except NotImplementedError:
+        pass
+    return HttpResponse("ok") # $ HttpResponse

python/ql/test/library-tests/frameworks/django-v2-v3/testproj/settings.py

@@ -74,12 +74,12 @@
 # Database
 # https://docs.djangoproject.com/en/3.1/ref/settings/#databases
 
-# DATABASES = {
-#     'default': {
-#         'ENGINE': 'django.db.backends.sqlite3',
-#         'NAME': BASE_DIR / 'db.sqlite3',
-#     }
-# }
+DATABASES = {
+    'default': {
+        'ENGINE': 'django.db.backends.sqlite3',
+        'NAME': BASE_DIR / 'db.sqlite3',
+    }
+}
 
 
 # Password validation