Skip to content

Commit 6bf46fd

Browse files
committed
Security fixes for ontology app SACGF/variantgrid_private#3824
H1: Encode gene_symbol in PanelApp OntologyBuilder cache key (panel_app_ontology.py) H2: Validate gene_symbol format before cached GeneDiseaseRelationshipView response H3: Remove dead urllib.parse.quote call whose result was never assigned (views_rest.py) M2: get_from_slug raises Http404 instead of unhandled DoesNotExist -> 500 M3: Validate forwarded ontology_service against OntologyService enum in autocomplete M4: Catch ValueError from OntologyService() in ontology_term_text -> 404 M5: Narrow bare except Exception to except ValueError in get_or_stub M6: Reject inputs >200 chars in OntologyIdNormalized.normalize
1 parent 9b2710b commit 6bf46fd

5 files changed

Lines changed: 30 additions & 8 deletions

File tree

ontology/models/models_ontology.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from django.contrib.postgres.fields import ArrayField
1717
from django.db import models, connection
1818
from django.db.models import PROTECT, CASCADE, QuerySet, Q, Max, TextChoices
19+
from django.http import Http404
1920
from django.urls import reverse
2021
from model_utils.models import TimeStampedModel, now
2122
from psqlextra.models import PostgresPartitionedModel
@@ -288,6 +289,8 @@ def num_part_safe(self) -> int:
288289

289290
@staticmethod
290291
def normalize(dirty_id: str) -> 'OntologyIdNormalized':
292+
if len(dirty_id) > 200:
293+
raise ValueError(f"Input too long ({len(dirty_id)} chars) to normalize as an ontology ID")
291294
parts = re.split("[:|_]", dirty_id)
292295
if len(parts) != 2:
293296
raise ValueError(f"Can not convert {dirty_id} to a proper id")
@@ -431,7 +434,10 @@ def url_safe_id(self):
431434
@staticmethod
432435
def get_from_slug(slug_pk):
433436
pk = slug_pk.replace("_", ":")
434-
return OntologyTerm.objects.get(pk=pk)
437+
try:
438+
return OntologyTerm.objects.get(pk=pk)
439+
except OntologyTerm.DoesNotExist:
440+
raise Http404
435441

436442
@staticmethod
437443
def get_gene_symbol(gene_symbol: Union[str, GeneSymbol]) -> 'OntologyTerm':
@@ -507,7 +513,7 @@ def get_or_stub(id_str: Union[str, OntologyIdNormalized]) -> 'OntologyTerm':
507513
return existing
508514
try:
509515
index_num_part_value = normal_id.num_part
510-
except Exception:
516+
except ValueError:
511517
index_num_part_value = normal_id.num_part_safe # Ontologies like MedGen can have alpha characters in the "index", providing an index of 0 until we update the model
512518
return OntologyTerm(
513519
id=normal_id.full_id,

ontology/panel_app_ontology.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import re
2+
import urllib.parse
23
from collections import defaultdict
34
from dataclasses import dataclass
45
from datetime import timedelta
@@ -90,7 +91,7 @@ def is_empty_results(data: Any):
9091
try:
9192
hgnc_term = OntologyTerm.get_gene_symbol(gene_symbol)
9293
panel_app = PanelAppServer.australia_instance()
93-
filename = panel_app.url + PANEL_APP_SEARCH_BY_GENES_BASE_PATH + gene_symbol
94+
filename = panel_app.url + PANEL_APP_SEARCH_BY_GENES_BASE_PATH + urllib.parse.quote(gene_symbol, safe="")
9495
ontology_builder = OntologyBuilder(filename=filename, context=str(gene_symbol),
9596
import_source=OntologyImportSource.PANEL_APP_AU,
9697
processor_version=PANEL_APP_API_PROCESSOR_VERSION,

ontology/views.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from django.contrib import messages
2+
from django.http import Http404
23
from django.shortcuts import get_object_or_404, redirect
34
from django.views.generic import TemplateView
45

@@ -11,7 +12,10 @@
1112

1213
def ontology_term_text(request, ontology_service, name):
1314
""" Occasionally we have service + name but not the ID - this is a way of building an URL for that """
14-
ontology_service = OntologyService(ontology_service) # Ensure valid
15+
try:
16+
ontology_service = OntologyService(ontology_service)
17+
except ValueError:
18+
raise Http404
1519
term = get_object_or_404(OntologyTerm, name=name, ontology_service=ontology_service)
1620
return redirect(term)
1721

ontology/views_autocomplete.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ def get_user_queryset(self, user):
4646
class OntologyTermAutocompleteView(AbstractOntologyTermAutocompleteView):
4747
def _get_ontology_service(self):
4848
# Passed ontology_service in forward
49-
return self.forwarded.get('ontology_service')
49+
value = self.forwarded.get('ontology_service')
50+
if value is None:
51+
return None
52+
try:
53+
return OntologyService(value)
54+
except ValueError:
55+
return None
5056

5157

5258
@method_decorator(cache_page(HOUR_SECS), name='dispatch')

ontology/views_rest.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import urllib
1+
import re
22

3+
from django.http import Http404
34
from django.utils.decorators import method_decorator
45
from django.views.decorators.cache import cache_page
56
from rest_framework.response import Response
@@ -14,14 +15,15 @@
1415
from ontology.ontology_matching import OntologyMatching
1516
from ontology.serializers import OntologyTermRelationSerializer
1617

18+
_GENE_SYMBOL_RE = re.compile(r'^[A-Za-z0-9\-]+$')
19+
1720

1821
class SearchMondoText(APIView):
1922
def get(self, request, **kwargs) -> Response:
2023

2124
search_term = request.GET.get('search_term') or ''
2225
gene_symbol = request.GET.get('gene_symbol')
2326

24-
urllib.parse.quote(search_term).replace('/', '%252F') # a regular escape / gets confused for a URL divider
2527
selected = [term.strip() for term in (request.GET.get('selected') or '').split(",") if term.strip()]
2628

2729
ontology_matches = OntologyMatching.from_search(search_text=search_term, gene_symbol=gene_symbol, selected=selected)
@@ -58,8 +60,11 @@ def get(self, request, *args, **kwargs):
5860
@method_decorator(cache_page(WEEK_SECS), name='get')
5961
class GeneDiseaseRelationshipView(APIView):
6062
def get(self, request, *args, **kwargs):
63+
gene_symbol = self.kwargs['gene_symbol']
64+
if not _GENE_SYMBOL_RE.match(gene_symbol):
65+
raise Http404
6166
data = []
6267
ontology_version = OntologyVersion.latest()
63-
for otr in ontology_version.gene_disease_relations(self.kwargs['gene_symbol'], quality_filter=ONTOLOGY_RELATIONSHIP_MEDIUM_QUALITY_FILTER):
68+
for otr in ontology_version.gene_disease_relations(gene_symbol, quality_filter=ONTOLOGY_RELATIONSHIP_MEDIUM_QUALITY_FILTER):
6469
data.append(OntologyTermRelationSerializer(otr).data)
6570
return Response(data)

0 commit comments

Comments
 (0)