From f1307b772a2728de27e2012c7ac8f0b4d47451b2 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 2 Nov 2021 10:45:58 +0100 Subject: [PATCH 01/21] Python: Add RequestHandler meta query --- python/ql/src/meta/alerts/RequestHandlers.ql | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 python/ql/src/meta/alerts/RequestHandlers.ql diff --git a/python/ql/src/meta/alerts/RequestHandlers.ql b/python/ql/src/meta/alerts/RequestHandlers.ql new file mode 100644 index 000000000000..2f79a3f1b352 --- /dev/null +++ b/python/ql/src/meta/alerts/RequestHandlers.ql @@ -0,0 +1,23 @@ +/** + * @name Request Handlers + * @description HTTP Server Request Handlers + * @kind problem + * @problem.severity recommendation + * @id py/meta/alerts/request-handlers + * @tags meta + * @precision very-low + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.Concepts +private import meta.MetaMetrics + +from HTTP::Server::RequestHandler requestHandler, string title +where + not requestHandler.getLocation().getFile() instanceof IgnoredFile and + if requestHandler.isMethod() + then + title = "Method " + requestHandler.getScope().(Class).getName() + "." + requestHandler.getName() + else title = requestHandler.toString() +select requestHandler, "RequestHandler: " + title From 9d843153d4b10e6c6528d5cc8f29665fe76d43f8 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 27 Oct 2021 13:16:28 +0200 Subject: [PATCH 02/21] Python: Set up test for Django REST framework this is just pure Django project for now, (and very much a copy of the one in `django-v2-v3`), to make it easier to see the changes needed to set up Django REST framework. --- .../frameworks/rest_framework/README.md | 1 + .../frameworks/rest_framework/manage.py | 22 +++ .../rest_framework/testapp/__init__.py | 0 .../rest_framework/testapp/admin.py | 3 + .../frameworks/rest_framework/testapp/apps.py | 6 + .../testapp/migrations/__init__.py | 0 .../rest_framework/testapp/models.py | 3 + .../rest_framework/testapp/tests.py | 3 + .../frameworks/rest_framework/testapp/urls.py | 7 + .../rest_framework/testapp/views.py | 4 + .../rest_framework/testproj/__init__.py | 0 .../rest_framework/testproj/asgi.py | 16 +++ .../rest_framework/testproj/settings.py | 126 ++++++++++++++++++ .../rest_framework/testproj/urls.py | 22 +++ .../rest_framework/testproj/wsgi.py | 16 +++ 15 files changed, 229 insertions(+) create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/README.md create mode 100755 python/ql/test/library-tests/frameworks/rest_framework/manage.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/__init__.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/apps.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/__init__.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/tests.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testproj/__init__.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testproj/asgi.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testproj/urls.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testproj/wsgi.py diff --git a/python/ql/test/library-tests/frameworks/rest_framework/README.md b/python/ql/test/library-tests/frameworks/rest_framework/README.md new file mode 100644 index 000000000000..72782d97e55e --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/README.md @@ -0,0 +1 @@ +See README for `django-v2-v3` which described how the project was set up and how to run the development server. diff --git a/python/ql/test/library-tests/frameworks/rest_framework/manage.py b/python/ql/test/library-tests/frameworks/rest_framework/manage.py new file mode 100755 index 000000000000..0e1a0b64a6e6 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/manage.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python +"""Django's command-line utility for administrative tasks.""" +import os +import sys + + +def main(): + """Run administrative tasks.""" + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'testproj.settings') + try: + from django.core.management import execute_from_command_line + except ImportError as exc: + raise ImportError( + "Couldn't import Django. Are you sure it's installed and " + "available on your PYTHONPATH environment variable? Did you " + "forget to activate a virtual environment?" + ) from exc + execute_from_command_line(sys.argv) + + +if __name__ == '__main__': + main() diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/__init__.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py new file mode 100644 index 000000000000..8c38f3f3dad5 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py @@ -0,0 +1,3 @@ +from django.contrib import admin + +# Register your models here. diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/apps.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/apps.py new file mode 100644 index 000000000000..8adcb9de0a0b --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class TestappConfig(AppConfig): + default_auto_field = 'django.db.models.BigAutoField' + name = 'testapp' diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/__init__.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py new file mode 100644 index 000000000000..71a836239075 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py @@ -0,0 +1,3 @@ +from django.db import models + +# Create your models here. diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/tests.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/tests.py new file mode 100644 index 000000000000..7ce503c2dd97 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/tests.py @@ -0,0 +1,3 @@ +from django.test import TestCase + +# Create your tests here. diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py new file mode 100644 index 000000000000..af668204542b --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py @@ -0,0 +1,7 @@ +from django.urls import path + +from . import views + +urlpatterns = [ + path("foo/", views.foo), # $routeSetup="foo/" +] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py new file mode 100644 index 000000000000..6bd3745add3c --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py @@ -0,0 +1,4 @@ +from django.http import HttpResponse, HttpRequest + +def foo(request: HttpRequest): + return HttpResponse("foo") diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/__init__.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/asgi.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/asgi.py new file mode 100644 index 000000000000..33e65481deea --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testproj/asgi.py @@ -0,0 +1,16 @@ +""" +ASGI config for testproj project. + +It exposes the ASGI callable as a module-level variable named ``application``. + +For more information on this file, see +https://docs.djangoproject.com/en/3.2/howto/deployment/asgi/ +""" + +import os + +from django.core.asgi import get_asgi_application + +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'testproj.settings') + +application = get_asgi_application() diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py new file mode 100644 index 000000000000..00624e7b8a93 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py @@ -0,0 +1,126 @@ +""" +Django settings for testproj project. + +Generated by 'django-admin startproject' using Django 3.2.8. + +For more information on this file, see +https://docs.djangoproject.com/en/3.2/topics/settings/ + +For the full list of settings and their values, see +https://docs.djangoproject.com/en/3.2/ref/settings/ +""" + +from pathlib import Path + +# Build paths inside the project like this: BASE_DIR / 'subdir'. +BASE_DIR = Path(__file__).resolve().parent.parent # $ getAPathArgument=Path(..) + + +# Quick-start development settings - unsuitable for production +# See https://docs.djangoproject.com/en/3.2/howto/deployment/checklist/ + +# SECURITY WARNING: keep the secret key used in production secret! +SECRET_KEY = 'django-insecure-hqg4$wqk3894#_4p$ibwzpg5+&dvx)%6q45v0yq=-43c886(($' + +# SECURITY WARNING: don't run with debug turned on in production! +DEBUG = True + +ALLOWED_HOSTS = [] + + +# Application definition + +INSTALLED_APPS = [ + 'testapp.apps.TestappConfig', + 'django.contrib.admin', + 'django.contrib.auth', + 'django.contrib.contenttypes', + 'django.contrib.sessions', + 'django.contrib.messages', + 'django.contrib.staticfiles', +] + +MIDDLEWARE = [ + 'django.middleware.security.SecurityMiddleware', + 'django.contrib.sessions.middleware.SessionMiddleware', + 'django.middleware.common.CommonMiddleware', + 'django.middleware.csrf.CsrfViewMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django.contrib.messages.middleware.MessageMiddleware', + 'django.middleware.clickjacking.XFrameOptionsMiddleware', +] + +ROOT_URLCONF = 'testproj.urls' + +TEMPLATES = [ + { + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + 'DIRS': [], + 'APP_DIRS': True, + 'OPTIONS': { + 'context_processors': [ + 'django.template.context_processors.debug', + 'django.template.context_processors.request', + 'django.contrib.auth.context_processors.auth', + 'django.contrib.messages.context_processors.messages', + ], + }, + }, +] + +WSGI_APPLICATION = 'testproj.wsgi.application' + + +# Database +# https://docs.djangoproject.com/en/3.2/ref/settings/#databases + +# DATABASES = { +# 'default': { +# 'ENGINE': 'django.db.backends.sqlite3', +# 'NAME': BASE_DIR / 'db.sqlite3', +# } +# } + + +# Password validation +# https://docs.djangoproject.com/en/3.2/ref/settings/#auth-password-validators + +AUTH_PASSWORD_VALIDATORS = [ + { + 'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator', + }, + { + 'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator', + }, + { + 'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator', + }, + { + 'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator', + }, +] + + +# Internationalization +# https://docs.djangoproject.com/en/3.2/topics/i18n/ + +LANGUAGE_CODE = 'en-us' + +TIME_ZONE = 'UTC' + +USE_I18N = True + +USE_L10N = True + +USE_TZ = True + + +# Static files (CSS, JavaScript, Images) +# https://docs.djangoproject.com/en/3.2/howto/static-files/ + +STATIC_URL = '/static/' + +# Default primary key field type +# https://docs.djangoproject.com/en/3.2/ref/settings/#default-auto-field + +DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/urls.py new file mode 100644 index 000000000000..f8d037cdcf92 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testproj/urls.py @@ -0,0 +1,22 @@ +"""testproj URL Configuration + +The `urlpatterns` list routes URLs to views. For more information please see: + https://docs.djangoproject.com/en/3.2/topics/http/urls/ +Examples: +Function views + 1. Add an import: from my_app import views + 2. Add a URL to urlpatterns: path('', views.home, name='home') +Class-based views + 1. Add an import: from other_app.views import Home + 2. Add a URL to urlpatterns: path('', Home.as_view(), name='home') +Including another URLconf + 1. Import the include() function: from django.urls import include, path + 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) +""" +from django.contrib import admin +from django.urls import path, include + +urlpatterns = [ + path('admin/', admin.site.urls), # $ routeSetup="admin/" + path("", include("testapp.urls")), # $routeSetup="" +] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/wsgi.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/wsgi.py new file mode 100644 index 000000000000..4103eb6492d6 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testproj/wsgi.py @@ -0,0 +1,16 @@ +""" +WSGI config for testproj project. + +It exposes the WSGI callable as a module-level variable named ``application``. + +For more information on this file, see +https://docs.djangoproject.com/en/3.2/howto/deployment/wsgi/ +""" + +import os + +from django.core.wsgi import get_wsgi_application + +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'testproj.settings') + +application = get_wsgi_application() From 9bbf08ddcfd76e8fd26f5e5c3e3b6ec303b12b1a Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 27 Oct 2021 14:15:59 +0200 Subject: [PATCH 03/21] Python: Add simple Django REST framework code --- .../frameworks/rest_framework/.gitignore | 1 + .../frameworks/rest_framework/README.md | 24 +++++++++++++- .../rest_framework/testapp/admin.py | 5 +++ .../testapp/migrations/0001_initial.py | 31 +++++++++++++++++++ .../testapp/migrations/0002_add_dummy_data.py | 29 +++++++++++++++++ .../rest_framework/testapp/models.py | 10 ++++++ .../rest_framework/testapp/serializers.py | 14 +++++++++ .../frameworks/rest_framework/testapp/urls.py | 12 +++++-- .../rest_framework/testapp/views.py | 22 +++++++++++-- .../rest_framework/testproj/settings.py | 13 ++++---- 10 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/.gitignore create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0001_initial.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0002_add_dummy_data.py create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/testapp/serializers.py diff --git a/python/ql/test/library-tests/frameworks/rest_framework/.gitignore b/python/ql/test/library-tests/frameworks/rest_framework/.gitignore new file mode 100644 index 000000000000..49ef2557b16e --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/.gitignore @@ -0,0 +1 @@ +db.sqlite3 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/README.md b/python/ql/test/library-tests/frameworks/rest_framework/README.md index 72782d97e55e..ca826010f3d5 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/README.md +++ b/python/ql/test/library-tests/frameworks/rest_framework/README.md @@ -1 +1,23 @@ -See README for `django-v2-v3` which described how the project was set up and how to run the development server. +See README for `django-v2-v3` which described how the project was set up. + +Since this test project uses models (and a DB), you generally need to run there 3 commands: + +``` +python manage.py makemigrations +python manage.py migrate +python manage.py runserver +``` + +Then visit http://127.0.0.1:8000/ + +# References + +- https://www.django-rest-framework.org/tutorial/quickstart/ + +# Editing data + +To edit data you should add an admin user (will prompt for password) + +``` +python manage.py createsuperuser --email admin@example.com --username admin +``` diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py index 8c38f3f3dad5..2bca18436258 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/admin.py @@ -1,3 +1,8 @@ +from .models import Foo, Bar + from django.contrib import admin # Register your models here. + +admin.site.register(Foo) +admin.site.register(Bar) diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0001_initial.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0001_initial.py new file mode 100644 index 000000000000..bc4ed0d1a41d --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0001_initial.py @@ -0,0 +1,31 @@ +# Generated by Django 3.2.8 on 2021-10-27 11:54 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='Foo', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=100)), + ('field_not_displayed', models.IntegerField()), + ], + ), + migrations.CreateModel( + name='Bar', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('n', models.IntegerField()), + ('foo', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='testapp.foo')), + ], + ), + ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0002_add_dummy_data.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0002_add_dummy_data.py new file mode 100644 index 000000000000..34c1cf473357 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/migrations/0002_add_dummy_data.py @@ -0,0 +1,29 @@ +# Generated by Django 3.2.8 on 2021-10-27 12:06 + +from django.db import migrations + +def add_dummy_data(apps, schema_editor): + Foo = apps.get_model("testapp", "Foo") + Bar = apps.get_model("testapp", "Bar") + + f1 = Foo(title="example 1", field_not_displayed=10) + f1.save() + f2 = Foo(title="example 2", field_not_displayed=20) + f2.save() + + b1 = Bar(n=42, foo=f1) + b1.save() + b2 = Bar(n=43, foo=f1) + b2.save() + b3 = Bar(n=1000, foo=f2) + b3.save() + +class Migration(migrations.Migration): + + dependencies = [ + ('testapp', '0001_initial'), + ] + + operations = [ + migrations.RunPython(add_dummy_data), + ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py index 71a836239075..34cdf073c17b 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/models.py @@ -1,3 +1,13 @@ from django.db import models # Create your models here. + + +class Foo(models.Model): + title = models.CharField(max_length=100) + field_not_displayed = models.IntegerField() + + +class Bar(models.Model): + n = models.IntegerField() + foo = models.ForeignKey(Foo, on_delete=models.PROTECT) diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/serializers.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/serializers.py new file mode 100644 index 000000000000..8c66ad918c6f --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/serializers.py @@ -0,0 +1,14 @@ +from .models import Foo, Bar +from rest_framework import serializers + + +class FooSerializer(serializers.HyperlinkedModelSerializer): + class Meta: + model = Foo + fields = ["title"] + + +class BarSerializer(serializers.HyperlinkedModelSerializer): + class Meta: + model = Bar + fields = ["n", "foo"] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py index af668204542b..6bde823b7bde 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py @@ -1,7 +1,15 @@ -from django.urls import path +from django.urls import path, include +from rest_framework import routers from . import views + +router = routers.DefaultRouter() +router.register(r"foos", views.FooViewSet) +router.register(r"bars", views.BarViewSet) + urlpatterns = [ - path("foo/", views.foo), # $routeSetup="foo/" + path("", include(router.urls)), + path("api-auth/", include("rest_framework.urls", namespace="rest_framework")), + path("example/", views.example), # $routeSetup="example/" ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py index 6bd3745add3c..b57fe293ce41 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py @@ -1,4 +1,20 @@ -from django.http import HttpResponse, HttpRequest +from .models import Foo, Bar +from .serializers import FooSerializer, BarSerializer + +from rest_framework import viewsets + + +class FooViewSet(viewsets.ModelViewSet): + queryset = Foo.objects.all() + serializer_class = FooSerializer -def foo(request: HttpRequest): - return HttpResponse("foo") + +class BarViewSet(viewsets.ModelViewSet): + queryset = Bar.objects.all() + serializer_class = BarSerializer + + +# this is pure django +from django.http import HttpResponse, HttpRequest +def example(request: HttpRequest): + return HttpResponse("example") diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py b/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py index 00624e7b8a93..b9a5f4493fb1 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testproj/settings.py @@ -38,6 +38,7 @@ 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', + 'rest_framework', ] MIDDLEWARE = [ @@ -74,12 +75,12 @@ # Database # https://docs.djangoproject.com/en/3.2/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 From 095f896f95f2fe002b2b22edee5aededf6b94025 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 27 Oct 2021 15:20:51 +0200 Subject: [PATCH 04/21] Python: Add examples of class/function based views --- .../frameworks/rest_framework/testapp/urls.py | 3 +- .../rest_framework/testapp/views.py | 32 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py index 6bde823b7bde..c83d5b4ca1ce 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py @@ -11,5 +11,6 @@ urlpatterns = [ path("", include(router.urls)), path("api-auth/", include("rest_framework.urls", namespace="rest_framework")), - path("example/", views.example), # $routeSetup="example/" + path("class-based-view/", views.MyClass.as_view()), # $routeSetup="lcass-based-view/" + path("function-based-view/", views.function_based_view), # $routeSetup="function-based-view/" ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py index b57fe293ce41..3f1dc78c305a 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py @@ -2,8 +2,13 @@ from .serializers import FooSerializer, BarSerializer from rest_framework import viewsets +from rest_framework.decorators import api_view +from rest_framework.views import APIView +from rest_framework.request import Request +from rest_framework.response import Response - +# Viewsets +# see https://www.django-rest-framework.org/tutorial/quickstart/ class FooViewSet(viewsets.ModelViewSet): queryset = Foo.objects.all() serializer_class = FooSerializer @@ -13,8 +18,25 @@ class BarViewSet(viewsets.ModelViewSet): queryset = Bar.objects.all() serializer_class = BarSerializer +# class based view +# see https://www.django-rest-framework.org/api-guide/views/#class-based-views + +class MyClass(APIView): + def initial(self, request, *args, **kwargs): + # this method will be called before processing any request + super().initial(request, *args, **kwargs) + + def get(self, request): + return Response("GET request") + + def post(self, request): + return Response("POST request") + + +# function based view +# see https://www.django-rest-framework.org/api-guide/views/#function-based-views + -# this is pure django -from django.http import HttpResponse, HttpRequest -def example(request: HttpRequest): - return HttpResponse("example") +@api_view(["GET", "POST"]) +def function_based_view(request: Request): + return Response({"message": "Hello, world!"}) From 75e2555a8aa3e2a61f363043b8cbec1ec63eb573 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 28 Oct 2021 19:20:18 +0200 Subject: [PATCH 05/21] Python: Add rest_framework taint tests --- .../rest_framework/ConceptsTest.expected | 0 .../frameworks/rest_framework/ConceptsTest.ql | 2 + .../rest_framework/InlineTaintTest.expected | 3 + .../rest_framework/InlineTaintTest.ql | 1 + .../frameworks/rest_framework/taint_test.py | 119 ++++++++++++++++++ 5 files changed, 125 insertions(+) create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.expected create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.ql create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.expected create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.ql create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/taint_test.py diff --git a/python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.expected b/python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.ql b/python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.ql new file mode 100644 index 000000000000..b557a0bccb69 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/ConceptsTest.ql @@ -0,0 +1,2 @@ +import python +import experimental.meta.ConceptsTest diff --git a/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.expected b/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.expected new file mode 100644 index 000000000000..79d760d87f42 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.expected @@ -0,0 +1,3 @@ +argumentToEnsureNotTaintedNotMarkedAsSpurious +untaintedArgumentToEnsureTaintedNotMarkedAsMissing +failures diff --git a/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.ql b/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.ql new file mode 100644 index 000000000000..027ad8667be6 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/InlineTaintTest.ql @@ -0,0 +1 @@ +import experimental.meta.InlineTaintTest diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py new file mode 100644 index 000000000000..270384a51ae5 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -0,0 +1,119 @@ +from rest_framework.decorators import api_view, parser_classes +from rest_framework.views import APIView +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.parsers import JSONParser + +from django.urls import path + +ensure_tainted = ensure_not_tainted = print + +# function based view +# see https://www.django-rest-framework.org/api-guide/views/#function-based-views + + +@api_view(["POST"]) +@parser_classes([JSONParser]) +def test_taint(request: Request, routed_param): # $ requestHandler routedParameter=routed_param + ensure_tainted(routed_param) # $ tainted + + ensure_tainted(request) # $ tainted + + # Has all the standard attributes of a django HttpRequest + # see https://github.com/encode/django-rest-framework/blob/00cd4ef864a8bf6d6c90819a983017070f9f08a5/rest_framework/request.py#L410-L418 + ensure_tainted(request.resolve_match.args) # $ MISSING: tainted + + # special new attributes added, see https://www.django-rest-framework.org/api-guide/requests/ + ensure_tainted( + request.data, # $ MISSING: tainted + request.data["key"], # $ MISSING: tainted + + # alias for .GET + request.query_params, # $ MISSING: tainted + request.query_params["key"], # $ MISSING: tainted + request.query_params.get("key"), # $ MISSING: tainted + request.query_params.getlist("key"), # $ MISSING: tainted + request.query_params.getlist("key")[0], # $ MISSING: tainted + request.query_params.pop("key"), # $ MISSING: tainted + request.query_params.pop("key")[0], # $ MISSING: tainted + + # see more detailed tests of `request.user` below + request.user, # $ MISSING: tainted + + request.auth, # $ MISSING: tainted + + # seems much more likely attack vector than .method, so included + request.content_type, # $ tainted + + # file-like + request.stream, # $ MISSING: tainted + request.stream.read(), # $ MISSING: tainted + ) + + ensure_not_tainted( + # although these could technically be user-controlled, it seems more likely to lead to FPs than interesting results. + request.accepted_media_type, + request.method, # $ SPURIOUS: tainted + ) + + # -------------------------------------------------------------------------- + # request.user + # -------------------------------------------------------------------------- + # + # This will normally be an instance of django.contrib.auth.models.User + # (authenticated) so we assume that normally user-controlled fields such as + # username/email is user-controlled, but that password isn't (since it's a hash). + # see https://docs.djangoproject.com/en/3.2/ref/contrib/auth/#fields + ensure_tainted( + request.user.username, # $ MISSING: tainted + request.user.first_name, # $ MISSING: tainted + request.user.last_name, # $ MISSING: tainted + request.user.email, # $ MISSING: tainted + ) + ensure_not_tainted(request.user.password) + + return Response("ok") + + +# class based view +# see https://www.django-rest-framework.org/api-guide/views/#class-based-views + + +class MyClass(APIView): + def initial(self, request, *args, **kwargs): + # this method will be called before processing any request + ensure_tainted(request) # $ MISSING: tainted + + def get(self, request: Request, routed_param): # $ requestHandler routedParameter=routed_param + ensure_tainted(routed_param) # $ tainted + + # request taint is the same as in function_based_view above + ensure_tainted( + request, # $ tainted + request.data # $ MISSING: tainted + ) + + # same as for standard Django view + ensure_tainted(self.args, self.kwargs) # $ tainted + + return Response("ok") + + + +# fake setup, you can't actually run this +urlpatterns = [ + path("test-taint/", test_taint), # $ routeSetup="test-taint/" + path("ClassView/", MyClass.as_view()), # $ routeSetup="ClassView/" +] + +# tests with no route-setup, but we can still tell that these are using Django REST +# framework + +@api_view(["POST"]) +def function_based_no_route(request: Request, possible_routed_param): + ensure_tainted(request, possible_routed_param) # $ MISSING: tainted + + +class ClassBasedNoRoute(APIView): + def get(self, request: Request, possible_routed_param): + ensure_tainted(request, possible_routed_param) # $ MISSING: tainted From a64e939d718b2e8704d0f0daef55ce441452c9d8 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 29 Oct 2021 11:28:16 +0200 Subject: [PATCH 06/21] Python: Add note about `.method` --- .../frameworks/rest_framework/taint_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index 270384a51ae5..d23f5b59509b 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -53,6 +53,15 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet ensure_not_tainted( # although these could technically be user-controlled, it seems more likely to lead to FPs than interesting results. request.accepted_media_type, + + # In normal Django, if you disable CSRF middleware, you're allowed to use custom + # HTTP methods, like `curl -X FOO `. + # However, with Django REST framework, doing that will yield: + # `{"detail":"Method \"FOO\" not allowed."}` + # + # In the end, since we model a Django REST framework request entirely as a + # extension of a Django request, we're not easily able to remove the taint from + # `.method`. request.method, # $ SPURIOUS: tainted ) From 222db37c0d503e54c07eed7b89ce5077ace431bc Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 29 Oct 2021 14:18:52 +0200 Subject: [PATCH 07/21] Python: Add initial rest_framework modeling I had to make the Django and PrivateDjango modeling non-private :O --- docs/codeql/support/reusables/frameworks.rst | 1 + python/ql/lib/semmle/python/Frameworks.qll | 1 + .../lib/semmle/python/frameworks/Django.qll | 8 ++- .../python/frameworks/RestFramework.qll | 68 +++++++++++++++++++ .../frameworks/rest_framework/taint_test.py | 8 +-- 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 python/ql/lib/semmle/python/frameworks/RestFramework.qll diff --git a/docs/codeql/support/reusables/frameworks.rst b/docs/codeql/support/reusables/frameworks.rst index 3eb05170f4a6..33a263552cca 100644 --- a/docs/codeql/support/reusables/frameworks.rst +++ b/docs/codeql/support/reusables/frameworks.rst @@ -155,6 +155,7 @@ Python built-in support Name, Category aiohttp.web, Web framework Django, Web framework + djangorestframework, Web framework Flask, Web framework Tornado, Web framework Twisted, Web framework diff --git a/python/ql/lib/semmle/python/Frameworks.qll b/python/ql/lib/semmle/python/Frameworks.qll index f8099d75e835..0b1237a5bf79 100644 --- a/python/ql/lib/semmle/python/Frameworks.qll +++ b/python/ql/lib/semmle/python/Frameworks.qll @@ -25,6 +25,7 @@ private import semmle.python.frameworks.MySQLdb private import semmle.python.frameworks.Peewee private import semmle.python.frameworks.Psycopg2 private import semmle.python.frameworks.PyMySQL +private import semmle.python.frameworks.RestFramework private import semmle.python.frameworks.Rsa private import semmle.python.frameworks.RuamelYaml private import semmle.python.frameworks.Simplejson diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index 89ff0537c975..90c845bd33bb 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -17,10 +17,12 @@ private import semmle.python.frameworks.internal.SelfRefMixin private import semmle.python.frameworks.internal.InstanceTaintStepsHelper /** + * INTERNAL: Do not use. + * * Provides models for the `django` PyPI package. * See https://www.djangoproject.com/. */ -private module Django { +module Django { /** Provides models for the `django.views` module */ module Views { /** @@ -466,10 +468,12 @@ private module Django { } /** + * INTERNAL: Do not use. + * * Provides models for the `django` PyPI package (that we are not quite ready to publicly expose yet). * See https://www.djangoproject.com/. */ -private module PrivateDjango { +module PrivateDjango { // --------------------------------------------------------------------------- // django // --------------------------------------------------------------------------- diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll new file mode 100644 index 000000000000..045035e6f51e --- /dev/null +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -0,0 +1,68 @@ +/** + * Provides classes modeling security-relevant aspects of the `djangorestframework` PyPI package + * (imported as `rest_framework`) + * + * See + * - https://www.django-rest-framework.org/ + * - https://pypi.org/project/djangorestframework/ + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.dataflow.new.RemoteFlowSources +private import semmle.python.dataflow.new.TaintTracking +private import semmle.python.Concepts +private import semmle.python.ApiGraphs +private import semmle.python.frameworks.internal.InstanceTaintStepsHelper +private import semmle.python.frameworks.Django +private import semmle.python.frameworks.Stdlib + +/** + * INTERNAL: Do not use. + * + * Provides models for the `djangorestframework` PyPI package + * (imported as `rest_framework`) + * + * See + * - https://www.django-rest-framework.org/ + * - https://pypi.org/project/djangorestframework/ + */ +private module RestFramework { + /** + * An `API::Node` representing the `rest_framework.views.APIView` class or any subclass + * that has explicitly been modeled in the CodeQL libraries. + */ + private class ModeledApiViewClasses extends Django::Views::View::ModeledSubclass { + ModeledApiViewClasses() { + this = API::moduleImport("rest_framework").getMember("views").getMember("APIView") + // TODO: Need to model all known subclasses + } + } + + /** + * A class that has a super-type which is a rest_framework APIView class, therefore also + * becoming a APIView class. + */ + class RestFrameworkApiViewClass extends PrivateDjango::DjangoViewClassFromSuperClass { + RestFrameworkApiViewClass() { + this.getABase() = any(ModeledApiViewClasses c).getASubclass*().getAUse().asExpr() + } + + override Function getARequestHandler() { + result = super.getARequestHandler() + or + // TODO: This doesn't handle attribute assignment. Should be OK, but analysis is not as complete as with + // points-to and `.lookup`, which would handle `post = my_post_handler` inside class def + result = this.getAMethod() and + result.getName() in [ + // these method names where found by looking through the APIView + // implementation in + // https://github.com/encode/django-rest-framework/blob/master/rest_framework/views.py#L104 + "initial", "http_method_not_allowed", "permission_denied", "throttled", + "get_authenticate_header", "perform_content_negotiation", "perform_authentication", + "check_permissions", "check_object_permissions", "check_throttles", "determine_version", + "initialize_request", "finalize_response", "dispatch", "options" + ] + } + } +} diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index d23f5b59509b..df4086a62aae 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -89,9 +89,9 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet class MyClass(APIView): - def initial(self, request, *args, **kwargs): + def initial(self, request, *args, **kwargs): # $ requestHandler # this method will be called before processing any request - ensure_tainted(request) # $ MISSING: tainted + ensure_tainted(request) # $ tainted def get(self, request: Request, routed_param): # $ requestHandler routedParameter=routed_param ensure_tainted(routed_param) # $ tainted @@ -124,5 +124,5 @@ def function_based_no_route(request: Request, possible_routed_param): class ClassBasedNoRoute(APIView): - def get(self, request: Request, possible_routed_param): - ensure_tainted(request, possible_routed_param) # $ MISSING: tainted + def get(self, request: Request, possible_routed_param): # $ requestHandler routedParameter=possible_routed_param + ensure_tainted(request, possible_routed_param) # $ tainted From 57e13c6066f0473c0d708a6e8b67e815e84008a4 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 29 Oct 2021 14:43:54 +0200 Subject: [PATCH 08/21] Python: `rest_framework.decorators.api_view` handling Had to expose even more things, and had to make the `DjangoRouteHandler` modeling more flexible so I could extend the char-pred in a different file. --- .../lib/semmle/python/frameworks/Django.qll | 22 ++++++---- .../python/frameworks/RestFramework.qll | 42 +++++++++++++++++++ .../frameworks/rest_framework/taint_test.py | 7 +++- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index 90c845bd33bb..fc6304f79f15 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -1898,13 +1898,7 @@ module PrivateDjango { * * Most functions take a django HttpRequest as a parameter (but not all). */ - private class DjangoRouteHandler extends Function { - DjangoRouteHandler() { - exists(DjangoRouteSetup route | route.getViewArg() = poorMansFunctionTracker(this)) - or - any(DjangoViewClass vc).getARequestHandler() = this - } - + class DjangoRouteHandler extends Function instanceof DjangoRouteHandler::Range { /** * Gets the index of the parameter where the first routed parameter can be passed -- * that is, the one just after any possible `self` or HttpRequest parameters. @@ -1924,6 +1918,18 @@ module PrivateDjango { Parameter getRequestParam() { result = this.getArg(this.getRequestParamIndex()) } } + module DjangoRouteHandler { + abstract class Range extends Function { } + + class StandardDjangoRouteHandlers extends Range { + StandardDjangoRouteHandlers() { + exists(DjangoRouteSetup route | route.getViewArg() = poorMansFunctionTracker(this)) + or + any(DjangoViewClass vc).getARequestHandler() = this + } + } + } + /** * A method named `get_redirect_url` on a django view class. * @@ -1945,7 +1951,7 @@ module PrivateDjango { } /** A data-flow node that sets up a route on a server, using the django framework. */ - abstract private class DjangoRouteSetup extends HTTP::Server::RouteSetup::Range, DataFlow::CfgNode { + abstract class DjangoRouteSetup extends HTTP::Server::RouteSetup::Range, DataFlow::CfgNode { /** Gets the data-flow node that is used as the argument for the view handler. */ abstract DataFlow::Node getViewArg(); diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 045035e6f51e..1fad3b0f3f2d 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -28,6 +28,9 @@ private import semmle.python.frameworks.Stdlib * - https://pypi.org/project/djangorestframework/ */ private module RestFramework { + // --------------------------------------------------------------------------- + // rest_framework.views.APIView handling + // --------------------------------------------------------------------------- /** * An `API::Node` representing the `rest_framework.views.APIView` class or any subclass * that has explicitly been modeled in the CodeQL libraries. @@ -65,4 +68,43 @@ private module RestFramework { ] } } + + // --------------------------------------------------------------------------- + // rest_framework.decorators.api_view handling + // --------------------------------------------------------------------------- + /** + * A function that is a request handler since it is decorated with `rest_framework.decorators.api_view` + */ + class RestFrameworkFunctionBasedView extends PrivateDjango::DjangoRouteHandler::Range { + RestFrameworkFunctionBasedView() { + this.getADecorator() = + API::moduleImport("rest_framework") + .getMember("decorators") + .getMember("api_view") + .getACall() + .asExpr() + } + } + + /** + * Ensuring that all `RestFrameworkFunctionBasedView` are also marked as a + * `HTTP::Server::RequestHandler`. We only need this for the ones that doesn't have a + * known route setup. + */ + class RestFrameworkFunctionBasedViewWithoutKnownRoute extends HTTP::Server::RequestHandler::Range, + PrivateDjango::DjangoRouteHandler instanceof RestFrameworkFunctionBasedView { + RestFrameworkFunctionBasedViewWithoutKnownRoute() { + not exists(PrivateDjango::DjangoRouteSetup setup | setup.getARequestHandler() = this) + } + + override Parameter getARoutedParameter() { + // Since we don't know the URL pattern, we simply mark all parameters as a routed + // parameter. This should give us more RemoteFlowSources but could also lead to + // more FPs. If this turns out to be the wrong tradeoff, we can always change our mind. + result in [this.getArg(_), this.getArgByName(_)] and + not result = any(int i | i < this.getFirstPossibleRoutedParamIndex() | this.getArg(i)) + } + + override string getFramework() { result = "Django (rest_framework)" } + } } diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index df4086a62aae..50a94a857f92 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -119,8 +119,11 @@ def get(self, request: Request, routed_param): # $ requestHandler routedParamete # framework @api_view(["POST"]) -def function_based_no_route(request: Request, possible_routed_param): - ensure_tainted(request, possible_routed_param) # $ MISSING: tainted +def function_based_no_route(request: Request, possible_routed_param): # $ requestHandler routedParameter=possible_routed_param + ensure_tainted( + request, # $ MISSING: tainted + possible_routed_param, # $ tainted + ) class ClassBasedNoRoute(APIView): From 5d77e62f3a8724c812c6e1ab32fb577338a85c87 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 29 Oct 2021 14:48:53 +0200 Subject: [PATCH 09/21] Python: Add basic rest_framework Request modeling --- .../python/frameworks/RestFramework.qll | 65 +++++++++++++++++++ .../frameworks/rest_framework/taint_test.py | 4 +- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 1fad3b0f3f2d..0948c4a0404d 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -107,4 +107,69 @@ private module RestFramework { override string getFramework() { result = "Django (rest_framework)" } } + + // --------------------------------------------------------------------------- + // request modeling + // --------------------------------------------------------------------------- + /** + * A parameter that will receive a `rest_framework.request.Request` instance when a + * request handler is invoked. + */ + private class RestFrameworkRequestHandlerRequestParam extends Request::InstanceSource, + RemoteFlowSource::Range, DataFlow::ParameterNode { + RestFrameworkRequestHandlerRequestParam() { + // rest_framework.views.APIView subclass + exists(RestFrameworkApiViewClass vc | + this.getParameter() = + vc.getARequestHandler().(PrivateDjango::DjangoRouteHandler).getRequestParam() + ) + or + // annotated with @api_view decorator + exists(PrivateDjango::DjangoRouteHandler rh | rh instanceof RestFrameworkFunctionBasedView | + this.getParameter() = rh.getRequestParam() + ) + } + + override string getSourceType() { result = "rest_framework.request.HttpRequest" } + } + + /** + * Provides models for the `rest_framework.request.Request` class + * + * See https://www.django-rest-framework.org/api-guide/requests/. + */ + module Request { + /** Gets a reference to the `rest_framework.request.Request` class. */ + private API::Node classRef() { + result = API::moduleImport("rest_framework").getMember("request").getMember("Request") + } + + /** + * A source of instances of `rest_framework.request.Request`, extend this class to model new instances. + * + * This can include instantiations of the class, return values from function + * calls, or a special parameter that will be set when functions are called by an external + * library. + * + * Use the predicate `Request::instance()` to get references to instances of `rest_framework.request.Request`. + */ + abstract class InstanceSource extends PrivateDjango::django::http::request::HttpRequest::InstanceSource { + } + + /** A direct instantiation of `rest_framework.request.Request`. */ + private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode { + ClassInstantiation() { this = classRef().getACall() } + } + + /** Gets a reference to an instance of `rest_framework.request.Request`. */ + private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { + t.start() and + result instanceof InstanceSource + or + exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) + } + + /** Gets a reference to an instance of `rest_framework.request.Request`. */ + DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } + } } diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index 50a94a857f92..97d6fa67c867 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -21,7 +21,7 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet # Has all the standard attributes of a django HttpRequest # see https://github.com/encode/django-rest-framework/blob/00cd4ef864a8bf6d6c90819a983017070f9f08a5/rest_framework/request.py#L410-L418 - ensure_tainted(request.resolve_match.args) # $ MISSING: tainted + ensure_tainted(request.resolver_match.args) # $ tainted # special new attributes added, see https://www.django-rest-framework.org/api-guide/requests/ ensure_tainted( @@ -121,7 +121,7 @@ def get(self, request: Request, routed_param): # $ requestHandler routedParamete @api_view(["POST"]) def function_based_no_route(request: Request, possible_routed_param): # $ requestHandler routedParameter=possible_routed_param ensure_tainted( - request, # $ MISSING: tainted + request, # $ tainted possible_routed_param, # $ tainted ) From 62d30630aa68b374d589eb3d851601cee99329e3 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 29 Oct 2021 14:52:03 +0200 Subject: [PATCH 10/21] Python: Add rest_framework Request taint modeling --- .../lib/semmle/python/frameworks/Django.qll | 46 +++++++++++++++++++ .../python/frameworks/RestFramework.qll | 41 +++++++++++++++++ .../frameworks/rest_framework/taint_test.py | 36 +++++++-------- 3 files changed, 105 insertions(+), 18 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index fc6304f79f15..5cae769fce0e 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -369,6 +369,52 @@ module Django { } } + /** + * Provides models for the `django.contrib.auth.models.User` class + * + * See https://docs.djangoproject.com/en/3.2/ref/contrib/auth/#user-model. + */ + module User { + /** + * A source of instances of `django.contrib.auth.models.User`, extend this class to model new instances. + * + * This can include instantiations of the class, return values from function + * calls, or a special parameter that will be set when functions are called by an external + * library. + * + * Use the predicate `User::instance()` to get references to instances of `django.contrib.auth.models.User`. + */ + abstract class InstanceSource extends DataFlow::LocalSourceNode { } + + /** Gets a reference to an instance of `django.contrib.auth.models.User`. */ + private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { + t.start() and + result instanceof InstanceSource + or + exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) + } + + /** Gets a reference to an instance of `django.contrib.auth.models.User`. */ + DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } + + /** + * Taint propagation for `django.contrib.auth.models.User`. + */ + private class InstanceTaintSteps extends InstanceTaintStepsHelper { + InstanceTaintSteps() { this = "django.contrib.auth.models.User" } + + override DataFlow::Node getInstance() { result = instance() } + + override string getAttributeName() { + result in ["username", "first_name", "last_name", "email"] + } + + override string getMethodName() { none() } + + override string getAsyncMethodName() { none() } + } + } + /** * Provides models for the `django.core.files.uploadedfile.UploadedFile` class * diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 0948c4a0404d..15fd70ddf78e 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -171,5 +171,46 @@ private module RestFramework { /** Gets a reference to an instance of `rest_framework.request.Request`. */ DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } + + /** + * Taint propagation for `rest_framework.request.Request`. + */ + private class InstanceTaintSteps extends InstanceTaintStepsHelper { + InstanceTaintSteps() { this = "rest_framework.request.Request" } + + override DataFlow::Node getInstance() { result = instance() } + + override string getAttributeName() { + result in ["data", "query_params", "user", "auth", "content_type", "stream"] + } + + override string getMethodName() { none() } + + override string getAsyncMethodName() { none() } + } + + /** An attribute read that is a `MultiValueDict` instance. */ + private class MultiValueDictInstances extends Django::MultiValueDict::InstanceSource { + MultiValueDictInstances() { + this.(DataFlow::AttrRead).getObject() = instance() and + this.(DataFlow::AttrRead).getAttributeName() = "query_params" + } + } + + /** An attribute read that is a `User` instance. */ + private class UserInstances extends Django::User::InstanceSource { + UserInstances() { + this.(DataFlow::AttrRead).getObject() = instance() and + this.(DataFlow::AttrRead).getAttributeName() = "user" + } + } + + /** An attribute read that is a file-like instance. */ + private class FileLikeInstances extends Stdlib::FileLikeObject::InstanceSource { + FileLikeInstances() { + this.(DataFlow::AttrRead).getObject() = instance() and + this.(DataFlow::AttrRead).getAttributeName() = "stream" + } + } } } diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index 97d6fa67c867..0094ec5ecaef 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -25,29 +25,29 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet # special new attributes added, see https://www.django-rest-framework.org/api-guide/requests/ ensure_tainted( - request.data, # $ MISSING: tainted - request.data["key"], # $ MISSING: tainted + request.data, # $ tainted + request.data["key"], # $ tainted # alias for .GET - request.query_params, # $ MISSING: tainted - request.query_params["key"], # $ MISSING: tainted - request.query_params.get("key"), # $ MISSING: tainted - request.query_params.getlist("key"), # $ MISSING: tainted - request.query_params.getlist("key")[0], # $ MISSING: tainted - request.query_params.pop("key"), # $ MISSING: tainted - request.query_params.pop("key")[0], # $ MISSING: tainted + request.query_params, # $ tainted + request.query_params["key"], # $ tainted + request.query_params.get("key"), # $ tainted + request.query_params.getlist("key"), # $ tainted + request.query_params.getlist("key")[0], # $ tainted + request.query_params.pop("key"), # $ tainted + request.query_params.pop("key")[0], # $ tainted # see more detailed tests of `request.user` below - request.user, # $ MISSING: tainted + request.user, # $ tainted - request.auth, # $ MISSING: tainted + request.auth, # $ tainted # seems much more likely attack vector than .method, so included request.content_type, # $ tainted # file-like - request.stream, # $ MISSING: tainted - request.stream.read(), # $ MISSING: tainted + request.stream, # $ tainted + request.stream.read(), # $ tainted ) ensure_not_tainted( @@ -74,10 +74,10 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet # username/email is user-controlled, but that password isn't (since it's a hash). # see https://docs.djangoproject.com/en/3.2/ref/contrib/auth/#fields ensure_tainted( - request.user.username, # $ MISSING: tainted - request.user.first_name, # $ MISSING: tainted - request.user.last_name, # $ MISSING: tainted - request.user.email, # $ MISSING: tainted + request.user.username, # $ tainted + request.user.first_name, # $ tainted + request.user.last_name, # $ tainted + request.user.email, # $ tainted ) ensure_not_tainted(request.user.password) @@ -99,7 +99,7 @@ def get(self, request: Request, routed_param): # $ requestHandler routedParamete # request taint is the same as in function_based_view above ensure_tainted( request, # $ tainted - request.data # $ MISSING: tainted + request.data # $ tainted ) # same as for standard Django view From 13815fe7282e14967e3f0e74f92a63b5917d5463 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Mon, 1 Nov 2021 11:47:22 +0100 Subject: [PATCH 11/21] Python: Model known APIView subclasses Added internal helper `.qll` file as well --- .../python/frameworks/RestFramework.qll | 50 ++++- .../frameworks/internal/SubclassFinder.qll | 189 ++++++++++++++++++ 2 files changed, 237 insertions(+), 2 deletions(-) create mode 100644 python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 15fd70ddf78e..36222358e150 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -37,8 +37,54 @@ private module RestFramework { */ private class ModeledApiViewClasses extends Django::Views::View::ModeledSubclass { ModeledApiViewClasses() { - this = API::moduleImport("rest_framework").getMember("views").getMember("APIView") - // TODO: Need to model all known subclasses + this = API::moduleImport("rest_framework").getMember("views").getMember("APIView") or + // imports generated by python/frameworks/internal/SubclassFinder.qll + this = + API::moduleImport("rest_framework") + .getMember("authtoken") + .getMember("views") + .getMember("APIView") or + this = + API::moduleImport("rest_framework") + .getMember("authtoken") + .getMember("views") + .getMember("ObtainAuthToken") or + this = API::moduleImport("rest_framework").getMember("decorators").getMember("APIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("CreateAPIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("DestroyAPIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("GenericAPIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("ListAPIView") or + this = + API::moduleImport("rest_framework").getMember("generics").getMember("ListCreateAPIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveAPIView") or + this = + API::moduleImport("rest_framework") + .getMember("generics") + .getMember("RetrieveDestroyAPIView") or + this = + API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveUpdateAPIView") or + this = + API::moduleImport("rest_framework") + .getMember("generics") + .getMember("RetrieveUpdateDestroyAPIView") or + this = API::moduleImport("rest_framework").getMember("generics").getMember("UpdateAPIView") or + this = API::moduleImport("rest_framework").getMember("routers").getMember("APIRootView") or + this = API::moduleImport("rest_framework").getMember("routers").getMember("SchemaView") or + this = + API::moduleImport("rest_framework") + .getMember("schemas") + .getMember("views") + .getMember("APIView") or + this = + API::moduleImport("rest_framework") + .getMember("schemas") + .getMember("views") + .getMember("SchemaView") or + this = API::moduleImport("rest_framework").getMember("viewsets").getMember("GenericViewSet") or + this = API::moduleImport("rest_framework").getMember("viewsets").getMember("ModelViewSet") or + this = + API::moduleImport("rest_framework").getMember("viewsets").getMember("ReadOnlyModelViewSet") or + this = API::moduleImport("rest_framework").getMember("viewsets").getMember("ViewSet") } } diff --git a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll new file mode 100644 index 000000000000..e09f2b457d6d --- /dev/null +++ b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll @@ -0,0 +1,189 @@ +/** + * INTERNAL: Do not use. + * + * Has predicates to help find subclasses in library code. Should only be used to aid in + * the manual library modeling process, + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.ApiGraphs +private import semmle.python.filters.Tests + +// very much inspired by the draft at https://github.com/github/codeql/pull/5632 +private module NotExposed { + // Instructions: + // This needs to be automated better, but for this prototype, here are some rough instructions: + // 1) fill out the `getAlreadyModeledClass` body below + // 2) quick-eval the `quickEvalMe` predicate below, and copy the output to your modeling predicate + class MySpec extends FindSubclassesSpec { + MySpec() { this = "MySpec" } + + override API::Node getAlreadyModeledClass() { + // FILL ME OUT ! (but don't commit with any changes) + none() + // for example + // result = API::moduleImport("rest_framework").getMember("views").getMember("APIView") + } + } + + predicate quickEvalMe(string newImport) { + newImport = + "// imports generated by python/frameworks/internal/SubclassFinder.qll\n" + "this = API::" + + concat(string newModelFullyQualified | + newModel(any(MySpec spec), newModelFullyQualified, _, _, _) + | + fullyQualifiedToAPIGraphPath(newModelFullyQualified), " or this = API::" + ) + } + + bindingset[fullyQaulified] + string fullyQualifiedToAPIGraphPath(string fullyQaulified) { + result = "moduleImport(\"" + fullyQaulified.replaceAll(".", "\").getMember(\"") + "\")" + } + + // -- Specs -- + bindingset[this] + abstract class FindSubclassesSpec extends string { + abstract API::Node getAlreadyModeledClass(); + } + + API::Node newOrExistingModeling(FindSubclassesSpec spec) { + result = spec.getAlreadyModeledClass() + or + exists(string newSubclassName | + newModel(spec, newSubclassName, _, _, _) and + result.getPath() = fullyQualifiedToAPIGraphPath(newSubclassName) + ) + } + + bindingset[fullyQualifiedName] + predicate alreadyModeled(FindSubclassesSpec spec, string fullyQualifiedName) { + fullyQualifiedToAPIGraphPath(fullyQualifiedName) = spec.getAlreadyModeledClass().getPath() + } + + predicate isNonTestProjectCode(AstNode ast) { + not ast.getScope*() instanceof TestScope and + not ast.getLocation().getFile().getRelativePath().matches("tests/%") and + exists(ast.getLocation().getFile().getRelativePath()) + } + + predicate hasAllStatement(Module mod) { + exists(AssignStmt a, GlobalVariable all | + a.defines(all) and + a.getScope() = mod and + all.getId() = "__all__" + ) + } + + /** + * Holds if `newAliasFullyQualified` describes new alias originating from the import + * `from import [as ]`, where `.` belongs to + * `spec`. + * So if this import happened in module `foo.bar`, `newAliasFullyQualified` would be + * `foo.bar.` (or `foo.bar.`). + * + * Note that this predicate currently respects `__all__` in sort of a backwards fashion. + * - if `__all__` is defined in module `foo.bar`, we only allow new aliases where the member name is also in `__all__`. (this doesn't map 100% to the semantics of imports though) + * - If `__all__` is not defined we don't impose any limitations. + * + * Also note that we don't currently consider deleting module-attributes at all, so in the code snippet below, we would consider that `my_module.foo` is a + * reference to `django.foo`, although `my_module.foo` isn't even available at runtime. (there currently also isn't any code to discover that `my_module.bar` + * is an alias to `django.foo`) + * ```py + * # module my_module + * from django import foo + * bar = foo + * del foo + * ``` + */ + predicate newDirectAlias( + FindSubclassesSpec spec, string newAliasFullyQualified, ImportMember importMember, Module mod, + Location loc + ) { + importMember = newOrExistingModeling(spec).getAUse().asExpr() and + importMember.getScope() = mod and + loc = importMember.getLocation() and + ( + mod.isPackageInit() and + newAliasFullyQualified = mod.getPackageName() + "." + importMember.getName() + or + not mod.isPackageInit() and + newAliasFullyQualified = mod.getName() + "." + importMember.getName() + ) and + ( + not hasAllStatement(mod) + or + mod.declaredInAll(importMember.getName()) + ) and + not alreadyModeled(spec, newAliasFullyQualified) and + isNonTestProjectCode(importMember) + } + + /** same as `newDirectAlias` predicate, but handling `from import *`, considering all ``, where `.` belongs to `spec`. */ + predicate newImportStar( + FindSubclassesSpec spec, string newAliasFullyQualified, ImportStar importStar, Module mod, + API::Node relevantClass, string relevantName, Location loc + ) { + relevantClass = newOrExistingModeling(spec) and + loc = importStar.getLocation() and + importStar.getScope() = mod and + // WHAT A HACK :D :D + relevantClass.getPath() = + relevantClass.getAPredecessor().getPath() + ".getMember(\"" + relevantName + "\")" and + relevantClass.getAPredecessor().getAUse().asExpr() = importStar.getModule() and + ( + mod.isPackageInit() and + newAliasFullyQualified = mod.getPackageName() + "." + relevantName + or + not mod.isPackageInit() and + newAliasFullyQualified = mod.getName() + "." + relevantName + ) and + ( + not hasAllStatement(mod) + or + mod.declaredInAll(relevantName) + ) and + not alreadyModeled(spec, newAliasFullyQualified) and + isNonTestProjectCode(importStar) + } + + /** Holds if `classExpr` defines a new subclass that belongs to `spec`, which has the fully qualified name `newSubclassQualified`. */ + predicate newSubclass( + FindSubclassesSpec spec, string newSubclassQualified, ClassExpr classExpr, Module mod, + Location loc + ) { + classExpr = newOrExistingModeling(spec).getASubclass*().getAUse().asExpr() and + classExpr.getScope() = mod and + newSubclassQualified = mod.getName() + "." + classExpr.getName() and + loc = classExpr.getLocation() and + not alreadyModeled(spec, newSubclassQualified) and + isNonTestProjectCode(classExpr) + } + + /** + * Holds if `newModelFullyQualified` describes either a new subclass, or a new alias, belonging to `spec` that we should include in our automated modeling. + * This new element is defined by `ast`, which is defined at `loc` in the module `mod`. + */ + query predicate newModel( + FindSubclassesSpec spec, string newModelFullyQualified, AstNode ast, Module mod, Location loc + ) { + ( + newSubclass(spec, newModelFullyQualified, ast, mod, loc) + or + newDirectAlias(spec, newModelFullyQualified, ast, mod, loc) + or + newImportStar(spec, newModelFullyQualified, ast, mod, _, _, loc) + ) + } + // inherint problem with API graphs is that there doesn't need to exist a result for all + // the stuff we have already modeled... as an example, the following query has no + // results when evaluated against Django + // + // select API::moduleImport("django") + // .getMember("contrib") + // .getMember("admin") + // .getMember("views") + // .getMember("main") + // .getMember("ChangeListSearchForm") +} From a7e4e5ef83fa56e3865f8e4f8f3d551368d335e6 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Mon, 1 Nov 2021 14:50:03 +0100 Subject: [PATCH 12/21] Python: Add rest_framework Response modeling --- .../python/frameworks/RestFramework.qll | 40 +++++++++++++++++++ .../rest_framework/response_test.py | 34 ++++++++++++++++ .../frameworks/rest_framework/taint_test.py | 4 +- .../frameworks/rest_framework/testapp/urls.py | 1 + .../rest_framework/testapp/views.py | 10 +++++ 5 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 python/ql/test/library-tests/frameworks/rest_framework/response_test.py diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 36222358e150..2e14a9496bf1 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -259,4 +259,44 @@ private module RestFramework { } } } + + // --------------------------------------------------------------------------- + // response modeling + // --------------------------------------------------------------------------- + /** + * Provides models for the `rest_framework.response.Response` class + * + * See https://www.django-rest-framework.org/api-guide/responses/. + */ + module Response { + /** Gets a reference to the `rest_framework.response.Response` class. */ + private API::Node classRef() { + result = API::moduleImport("rest_framework").getMember("response").getMember("Response") + } + + /** + * A source of instances of `rest_framework.response.Response`, extend this class to model new instances. + * + * This can include instantiations of the class, return values from function + * calls, or a special parameter that will be set when functions are called by an external + * library. + * + * Use the predicate `Response::instance()` to get references to instances of `rest_framework.response.Response`. + */ + abstract class InstanceSource extends DataFlow::LocalSourceNode { } + + /** A direct instantiation of `rest_framework.response.Response`. */ + private class ClassInstantiation extends PrivateDjango::django::http::response::HttpResponse::InstanceSource, + DataFlow::CallCfgNode { + ClassInstantiation() { this = classRef().getACall() } + + override DataFlow::Node getBody() { result in [this.getArg(0), this.getArgByName("data")] } + + override DataFlow::Node getMimetypeOrContentTypeArg() { + result in [this.getArg(5), this.getArgByName("content_type")] + } + + override string getMimetypeDefault() { none() } + } + } } diff --git a/python/ql/test/library-tests/frameworks/rest_framework/response_test.py b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py new file mode 100644 index 000000000000..ddf4325c48f3 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py @@ -0,0 +1,34 @@ +from rest_framework.decorators import api_view +from rest_framework.response import Response + +@api_view() +def normal_response(request): # $ requestHandler + # has no pre-defined content type, since that will be negotiated + # see https://www.django-rest-framework.org/api-guide/responses/ + data = "data" + resp = Response(data) # $ HttpResponse responseBody=data + return resp + +@api_view() +def plain_text_response(request): # $ requestHandler + # this response is not the standard way to use the Djagno REST framework, but it + # certainly is possible -- notice that the response contains double quotes + data = 'this response will contain double quotes since it was a string' + resp = Response(data, None, None, None, None, "text/plain") # $ HttpResponse mimetype=text/plain responseBody=data + resp = Response(data=data, content_type="text/plain") # $ HttpResponse mimetype=text/plain responseBody=data + return resp + +################################################################################ +# Cookies +################################################################################ + +@api_view +def setting_cookie(request): + resp = Response() # $ HttpResponse + resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value" + resp.set_cookie(key="key4", value="value") # $ CookieWrite CookieName="key4" CookieValue="value" + resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2" + resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3" + resp.delete_cookie("key4") # $ CookieWrite CookieName="key4" + resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4" + return resp diff --git a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py index 0094ec5ecaef..4a22e03b5638 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/taint_test.py @@ -81,7 +81,7 @@ def test_taint(request: Request, routed_param): # $ requestHandler routedParamet ) ensure_not_tainted(request.user.password) - return Response("ok") + return Response("ok") # $ HttpResponse responseBody="ok" # class based view @@ -105,7 +105,7 @@ def get(self, request: Request, routed_param): # $ requestHandler routedParamete # same as for standard Django view ensure_tainted(self.args, self.kwargs) # $ tainted - return Response("ok") + return Response("ok") # $ HttpResponse responseBody="ok" diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py index c83d5b4ca1ce..2b8f72303c2d 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py @@ -13,4 +13,5 @@ path("api-auth/", include("rest_framework.urls", namespace="rest_framework")), path("class-based-view/", views.MyClass.as_view()), # $routeSetup="lcass-based-view/" path("function-based-view/", views.function_based_view), # $routeSetup="function-based-view/" + path("cookie-test/", views.cookie_test), # $routeSetup="function-based-view/" ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py index 3f1dc78c305a..86b339cca581 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py @@ -40,3 +40,13 @@ def post(self, request): @api_view(["GET", "POST"]) def function_based_view(request: Request): return Response({"message": "Hello, world!"}) + + +@api_view(["GET", "POST"]) +def cookie_test(request: Request): + resp = Response("wat") + resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value" + resp.set_cookie(key="key4", value="value") # $ CookieWrite CookieName="key" CookieValue="value" + resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2" + resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3" + return resp From fd12b144bc8c46fd5262215333efc0f5e6a6956b Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Mon, 1 Nov 2021 14:50:55 +0100 Subject: [PATCH 13/21] Python: Add change-note --- .../change-notes/2021-10-29-django-REST-framework-modeling.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 python/change-notes/2021-10-29-django-REST-framework-modeling.md diff --git a/python/change-notes/2021-10-29-django-REST-framework-modeling.md b/python/change-notes/2021-10-29-django-REST-framework-modeling.md new file mode 100644 index 000000000000..7f7eebaf9fa7 --- /dev/null +++ b/python/change-notes/2021-10-29-django-REST-framework-modeling.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added modeling of HTTP requests and responses when using the Django REST Framework (`djangorestframework` PyPI package), which leads to additional remote flow sources. From 5c2734c6433e74dd729418d60320787558857269 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 2 Nov 2021 09:37:23 +0100 Subject: [PATCH 14/21] Python: Fix experimental Django.qll --- python/ql/src/experimental/semmle/python/frameworks/Django.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Django.qll b/python/ql/src/experimental/semmle/python/frameworks/Django.qll index 27ec7f6bd75e..55d3b4af35c7 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Django.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Django.qll @@ -10,7 +10,7 @@ private import experimental.semmle.python.Concepts private import semmle.python.ApiGraphs import semmle.python.dataflow.new.RemoteFlowSources -private module PrivateDjango { +private module ExperimentalPrivateDjango { private module django { API::Node http() { result = API::moduleImport("django").getMember("http") } From 83389be8e21e25542a867bc83a169b3f21c8cda5 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 2 Nov 2021 11:02:51 +0100 Subject: [PATCH 15/21] Python: Add some missing QLDocs --- .../ql/lib/semmle/python/frameworks/Django.qll | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll index 5cae769fce0e..9e66c728f6ee 100644 --- a/python/ql/lib/semmle/python/frameworks/Django.qll +++ b/python/ql/lib/semmle/python/frameworks/Django.qll @@ -546,6 +546,7 @@ module PrivateDjango { /** Gets a reference to the `django.db.connection` object. */ API::Node connection() { result = db().getMember("connection") } + /** A `django.db.connection` is a PEP249 compliant DB connection. */ class DjangoDbConnection extends PEP249::Connection::InstanceSource { DjangoDbConnection() { this = connection().getAUse() } } @@ -742,6 +743,7 @@ module PrivateDjango { /** Provides models for the `django.conf` module */ module conf { + /** Provides models for the `django.conf.urls` module */ module conf_urls { // ------------------------------------------------------------------------- // django.conf.urls @@ -940,6 +942,7 @@ module PrivateDjango { * See https://docs.djangoproject.com/en/3.1/ref/request-response/#django.http.HttpResponse. */ module HttpResponse { + /** Gets a reference to the `django.http.response.HttpResponse` class. */ API::Node baseClassRef() { result = response().getMember("HttpResponse") or @@ -947,7 +950,7 @@ module PrivateDjango { result = http().getMember("HttpResponse") } - /** Gets a reference to the `django.http.response.HttpResponse` class. */ + /** Gets a reference to the `django.http.response.HttpResponse` class or any subclass. */ API::Node classRef() { result = baseClassRef().getASubclass*() } /** @@ -1943,6 +1946,9 @@ module PrivateDjango { * with the django framework. * * Most functions take a django HttpRequest as a parameter (but not all). + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `DjangoRouteHandler::Range` instead. */ class DjangoRouteHandler extends Function instanceof DjangoRouteHandler::Range { /** @@ -1964,10 +1970,16 @@ module PrivateDjango { Parameter getRequestParam() { result = this.getArg(this.getRequestParamIndex()) } } + /** Provides a class for modeling new django route handlers. */ module DjangoRouteHandler { + /** + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `DjangoRouteHandler` instead. + */ abstract class Range extends Function { } - class StandardDjangoRouteHandlers extends Range { + /** Route handlers from normal usage of django. */ + private class StandardDjangoRouteHandlers extends Range { StandardDjangoRouteHandlers() { exists(DjangoRouteSetup route | route.getViewArg() = poorMansFunctionTracker(this)) or From f48ecb1dc83e645feb997768c05447f9bea7bd78 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 10:57:56 +0100 Subject: [PATCH 16/21] Python: Apply suggestions from code review Co-authored-by: yoff --- .../ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll index e09f2b457d6d..7a3319e16027 100644 --- a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll +++ b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll @@ -14,6 +14,7 @@ private import semmle.python.filters.Tests private module NotExposed { // Instructions: // This needs to be automated better, but for this prototype, here are some rough instructions: + // 0) get a database of the library you are about to model // 1) fill out the `getAlreadyModeledClass` body below // 2) quick-eval the `quickEvalMe` predicate below, and copy the output to your modeling predicate class MySpec extends FindSubclassesSpec { From 62e58b534c008376a3f1a7c014269834963d47e5 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 11:10:30 +0100 Subject: [PATCH 17/21] Python: SubclassFinder: reorder + comment --- .../frameworks/internal/SubclassFinder.qll | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll index 7a3319e16027..c5e509c833af 100644 --- a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll +++ b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll @@ -38,17 +38,56 @@ private module NotExposed { ) } + // --------------------------------------------------------------------------- + // Implementation below + // --------------------------------------------------------------------------- + // + // inherent problem with API graphs is that there doesn't need to exist a result for + // all the stuff we have already modeled... as an example, the following query has no + // results when evaluated against a django/django DB + // + // select API::moduleImport("django") + // .getMember("contrib") + // .getMember("admin") + // .getMember("views") + // .getMember("main") + // .getMember("ChangeListSearchForm") + // + // therefore we use fully qualified names to capture new classes/new aliases. + // + // note that this implementation was originally created to help with automatically + // modeling packages in mind, and was just copied for this purpose. See + // https://github.com/github/codeql/pull/5632 for more discussion. I wanted to get + // this into the codeql-repo, so it could be of use when modeling 3rd party libraries, + // and save some manual effort. + // + // bindingset[fullyQaulified] string fullyQualifiedToAPIGraphPath(string fullyQaulified) { result = "moduleImport(\"" + fullyQaulified.replaceAll(".", "\").getMember(\"") + "\")" } - // -- Specs -- bindingset[this] abstract class FindSubclassesSpec extends string { abstract API::Node getAlreadyModeledClass(); } + /** + * Holds if `newModelFullyQualified` describes either a new subclass, or a new alias, belonging to `spec` that we should include in our automated modeling. + * This new element is defined by `ast`, which is defined at `loc` in the module `mod`. + */ + query predicate newModel( + FindSubclassesSpec spec, string newModelFullyQualified, AstNode ast, Module mod, Location loc + ) { + ( + newSubclass(spec, newModelFullyQualified, ast, mod, loc) + or + newDirectAlias(spec, newModelFullyQualified, ast, mod, loc) + or + newImportStar(spec, newModelFullyQualified, ast, mod, _, _, loc) + ) + } + API::Node newOrExistingModeling(FindSubclassesSpec spec) { result = spec.getAlreadyModeledClass() or @@ -161,30 +200,4 @@ private module NotExposed { not alreadyModeled(spec, newSubclassQualified) and isNonTestProjectCode(classExpr) } - - /** - * Holds if `newModelFullyQualified` describes either a new subclass, or a new alias, belonging to `spec` that we should include in our automated modeling. - * This new element is defined by `ast`, which is defined at `loc` in the module `mod`. - */ - query predicate newModel( - FindSubclassesSpec spec, string newModelFullyQualified, AstNode ast, Module mod, Location loc - ) { - ( - newSubclass(spec, newModelFullyQualified, ast, mod, loc) - or - newDirectAlias(spec, newModelFullyQualified, ast, mod, loc) - or - newImportStar(spec, newModelFullyQualified, ast, mod, _, _, loc) - ) - } - // inherint problem with API graphs is that there doesn't need to exist a result for all - // the stuff we have already modeled... as an example, the following query has no - // results when evaluated against Django - // - // select API::moduleImport("django") - // .getMember("contrib") - // .getMember("admin") - // .getMember("views") - // .getMember("main") - // .getMember("ChangeListSearchForm") } From 5e4b866f2bae9ef241121acc5db7ae16ffbd3a32 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 11:37:54 +0100 Subject: [PATCH 18/21] Python: Model `rest_framework.exceptions.APIException` --- .../python/frameworks/RestFramework.qll | 47 +++++++++++++++++++ .../rest_framework/response_test.py | 16 +++++++ .../frameworks/rest_framework/testapp/urls.py | 1 + .../rest_framework/testapp/views.py | 7 +++ 4 files changed, 71 insertions(+) diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 2e14a9496bf1..a48f8d6ef6ab 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -299,4 +299,51 @@ private module RestFramework { override string getMimetypeDefault() { none() } } } + + // --------------------------------------------------------------------------- + // Exception response modeling + // --------------------------------------------------------------------------- + /** + * Provides models for the `rest_framework.exceptions.APIException` class and subclasses + * + * See https://www.django-rest-framework.org/api-guide/exceptions/#api-reference + */ + module APIException { + /** A direct instantiation of `rest_framework.exceptions.APIException` or subclass. */ + private class ClassInstantiation extends HTTP::Server::HttpResponse::Range, + DataFlow::CallCfgNode { + string className; + + ClassInstantiation() { + className in [ + "APIException", "ValidationError", "ParseError", "AuthenticationFailed", + "NotAuthenticated", "PermissionDenied", "NotFound", "MethodNotAllowed", "NotAcceptable", + "UnsupportedMediaType", "Throttled" + ] and + this = + API::moduleImport("rest_framework") + .getMember("exceptions") + .getMember(className) + .getACall() + } + + override DataFlow::Node getBody() { + className in [ + "APIException", "ValidationError", "ParseError", "AuthenticationFailed", + "NotAuthenticated", "PermissionDenied", "NotFound", "NotAcceptable" + ] and + result = this.getArg(0) + or + className in ["MethodNotAllowed", "UnsupportedMediaType", "Throttled"] and + result = this.getArg(1) + or + result = this.getArgByName("detail") + } + + // How to support the `headers` argument here? + override DataFlow::Node getMimetypeOrContentTypeArg() { none() } + + override string getMimetypeDefault() { none() } + } + } } diff --git a/python/ql/test/library-tests/frameworks/rest_framework/response_test.py b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py index ddf4325c48f3..ec093499df63 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/response_test.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py @@ -1,5 +1,6 @@ from rest_framework.decorators import api_view from rest_framework.response import Response +from rest_framework.exceptions import APIException @api_view() def normal_response(request): # $ requestHandler @@ -32,3 +33,18 @@ def setting_cookie(request): resp.delete_cookie("key4") # $ CookieWrite CookieName="key4" resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4" return resp + +################################################################################ +# Exceptions +################################################################################ + +# see https://www.django-rest-framework.org/api-guide/exceptions/ + +@api_view(["GET", "POST"]) +def exception_test(request): # $ requestHandler + data = "exception details" + # note: `code details` not exposed by default + code = "code details" + e1 = APIException(data, code) # $ HttpResponse responseBody=data + e2 = APIException(detail=data, code=code) # $ HttpResponse responseBody=data + raise e2 diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py index 2b8f72303c2d..856e8e031bba 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/urls.py @@ -14,4 +14,5 @@ path("class-based-view/", views.MyClass.as_view()), # $routeSetup="lcass-based-view/" path("function-based-view/", views.function_based_view), # $routeSetup="function-based-view/" path("cookie-test/", views.cookie_test), # $routeSetup="function-based-view/" + path("exception-test/", views.exception_test), # $routeSetup="exception-test/" ] diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py index 86b339cca581..47e304f9f7b5 100644 --- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py +++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py @@ -6,6 +6,7 @@ from rest_framework.views import APIView from rest_framework.request import Request from rest_framework.response import Response +from rest_framework.exceptions import APIException # Viewsets # see https://www.django-rest-framework.org/tutorial/quickstart/ @@ -50,3 +51,9 @@ def cookie_test(request: Request): resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2" resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3" return resp + +@api_view(["GET", "POST"]) +def exception_test(request: Request): + # see https://www.django-rest-framework.org/api-guide/exceptions/ + # note: `code details` not exposed by default + raise APIException("exception details", "code details") From f7b53321b92c0d206f4ed2d7f606ae8b54cdc4f3 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 13:19:20 +0100 Subject: [PATCH 19/21] Python: Remove copy-pasted comment --- python/ql/lib/semmle/python/frameworks/RestFramework.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index a48f8d6ef6ab..1aefbe4fd870 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -340,7 +340,6 @@ private module RestFramework { result = this.getArgByName("detail") } - // How to support the `headers` argument here? override DataFlow::Node getMimetypeOrContentTypeArg() { none() } override string getMimetypeDefault() { none() } From de69e4c6457bf9af3c0054610ee45fee2d555df1 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 13:29:03 +0100 Subject: [PATCH 20/21] Python: Expand on SubclassFinder implementation note --- .../frameworks/internal/SubclassFinder.qll | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll index c5e509c833af..36f03cd14012 100644 --- a/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll +++ b/python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll @@ -42,24 +42,30 @@ private module NotExposed { // Implementation below // --------------------------------------------------------------------------- // - // inherent problem with API graphs is that there doesn't need to exist a result for - // all the stuff we have already modeled... as an example, the following query has no - // results when evaluated against a django/django DB + // We are looking to find all subclassed of the already modelled classes, and ideally + // we would identify an `API::Node` for each (then `toString` would give the API + // path). // - // select API::moduleImport("django") - // .getMember("contrib") - // .getMember("admin") - // .getMember("views") - // .getMember("main") - // .getMember("ChangeListSearchForm") + // An inherent problem with API graphs is that there doesn't need to exist a result + // for the API graph path that we want to add to our modeling (the path to the new + // subclass). As an example, the following query has no results when evaluated against + // a django/django DB. // - // therefore we use fully qualified names to capture new classes/new aliases. + // select API::moduleImport("django") .getMember("contrib") .getMember("admin") + // .getMember("views") .getMember("main") .getMember("ChangeListSearchForm") // - // note that this implementation was originally created to help with automatically - // modeling packages in mind, and was just copied for this purpose. See - // https://github.com/github/codeql/pull/5632 for more discussion. I wanted to get - // this into the codeql-repo, so it could be of use when modeling 3rd party libraries, - // and save some manual effort. + // + // Since it is a Form subclass that we would want to capture for our Django modeling, + // we want to extend our modeling (that is written in a qll file) with exactly that + // piece of code, but since the API::Node doesn't exist, we can't select that from a + // predicate and print its path. We need a different approach, and for that we use + // fully qualified names to capture new classes/new aliases, and transform these into + // API paths (to be included in the modeling that is inserted into the `.qll` files), + // see `fullyQualifiedToAPIGraphPath`. + // + // NOTE: this implementation was originally created to help with automatically + // modeling packages in mind, and has been adjusted to help with manual library + // modeling. See https://github.com/github/codeql/pull/5632 for more discussion. // // bindingset[fullyQaulified] From 491f72bb2a6cf8746f7557b218feec32c1420370 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 12 Nov 2021 13:30:03 +0100 Subject: [PATCH 21/21] Python: Adjust generated code to be more familiar --- .../python/frameworks/RestFramework.qll | 63 ++++++++++++------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/RestFramework.qll b/python/ql/lib/semmle/python/frameworks/RestFramework.qll index 1aefbe4fd870..a6373df71208 100644 --- a/python/ql/lib/semmle/python/frameworks/RestFramework.qll +++ b/python/ql/lib/semmle/python/frameworks/RestFramework.qll @@ -37,53 +37,74 @@ private module RestFramework { */ private class ModeledApiViewClasses extends Django::Views::View::ModeledSubclass { ModeledApiViewClasses() { - this = API::moduleImport("rest_framework").getMember("views").getMember("APIView") or + this = API::moduleImport("rest_framework").getMember("views").getMember("APIView") + or // imports generated by python/frameworks/internal/SubclassFinder.qll this = API::moduleImport("rest_framework") .getMember("authtoken") .getMember("views") - .getMember("APIView") or + .getMember("APIView") + or this = API::moduleImport("rest_framework") .getMember("authtoken") .getMember("views") - .getMember("ObtainAuthToken") or - this = API::moduleImport("rest_framework").getMember("decorators").getMember("APIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("CreateAPIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("DestroyAPIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("GenericAPIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("ListAPIView") or + .getMember("ObtainAuthToken") + or + this = API::moduleImport("rest_framework").getMember("decorators").getMember("APIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("CreateAPIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("DestroyAPIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("GenericAPIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("ListAPIView") + or this = - API::moduleImport("rest_framework").getMember("generics").getMember("ListCreateAPIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveAPIView") or + API::moduleImport("rest_framework").getMember("generics").getMember("ListCreateAPIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveAPIView") + or this = API::moduleImport("rest_framework") .getMember("generics") - .getMember("RetrieveDestroyAPIView") or + .getMember("RetrieveDestroyAPIView") + or this = - API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveUpdateAPIView") or + API::moduleImport("rest_framework").getMember("generics").getMember("RetrieveUpdateAPIView") + or this = API::moduleImport("rest_framework") .getMember("generics") - .getMember("RetrieveUpdateDestroyAPIView") or - this = API::moduleImport("rest_framework").getMember("generics").getMember("UpdateAPIView") or - this = API::moduleImport("rest_framework").getMember("routers").getMember("APIRootView") or - this = API::moduleImport("rest_framework").getMember("routers").getMember("SchemaView") or + .getMember("RetrieveUpdateDestroyAPIView") + or + this = API::moduleImport("rest_framework").getMember("generics").getMember("UpdateAPIView") + or + this = API::moduleImport("rest_framework").getMember("routers").getMember("APIRootView") + or + this = API::moduleImport("rest_framework").getMember("routers").getMember("SchemaView") + or this = API::moduleImport("rest_framework") .getMember("schemas") .getMember("views") - .getMember("APIView") or + .getMember("APIView") + or this = API::moduleImport("rest_framework") .getMember("schemas") .getMember("views") - .getMember("SchemaView") or - this = API::moduleImport("rest_framework").getMember("viewsets").getMember("GenericViewSet") or - this = API::moduleImport("rest_framework").getMember("viewsets").getMember("ModelViewSet") or + .getMember("SchemaView") + or + this = API::moduleImport("rest_framework").getMember("viewsets").getMember("GenericViewSet") + or + this = API::moduleImport("rest_framework").getMember("viewsets").getMember("ModelViewSet") + or this = - API::moduleImport("rest_framework").getMember("viewsets").getMember("ReadOnlyModelViewSet") or + API::moduleImport("rest_framework").getMember("viewsets").getMember("ReadOnlyModelViewSet") + or this = API::moduleImport("rest_framework").getMember("viewsets").getMember("ViewSet") } }