fix(security): hardening z równoległego security review (6 findingów)#610
Open
mpasternak wants to merge 4 commits into
Open
fix(security): hardening z równoległego security review (6 findingów)#610mpasternak wants to merge 4 commits into
mpasternak wants to merge 4 commits into
Conversation
…hrottling DjangoQL Trzy poprawki z równoległego security review (attack-surface newralgiczny): - #2 stored-XSS: admin „Przemapowanie prac autora" budował HTML f-stringiem z nieescapowanym tytułem/źródłem/wydawnictwem publikacji (niezaufany HTML z importu/zgłoszenia) i mark_safe-ował go. Dodano escape() na wartościach. Ta sama klasa co wcześniejszy fix XSS w kolejce PBN, nieobjęta nim. - #3 IDOR: /api/v1/autor_jednostka/ zwracał powiązania zatrudnienia (jednostka, daty pracy, funkcja, PK autora) także dla autorów ukrytych (pokazuj=False), obchodząc pokazuj=False z AutorViewSet. Dodano filtr autor__pokazuj=True dla anonima (analogicznie do AutorViewSet — zalogowany widzi wszystkich). - #6 DoS: /api/v1/zapytanie/* (ciężki multi-join DjangoQL) nie miał throttlingu. Dodano SearchUserThrottle (endpointy wymagają logowania → throttle po userze). Repro-testy (TDD) dodane dla każdej luki. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oken) #1 (HIGH) z security review. Adminowa wyszukiwarka DjangoQL (BppQLSchema) nie ma allow-listy modeli (ma ją tylko podklasa BppQLSchemaOgraniczony używana przez API/web), więc DjangoQL rekurencyjnie schodził przez reverse-relację Autor.user do BppUser. Domyślny filtr djangoql wycina tylko pole `password`, więc pbn_token (żywy token PBN, CharField 128), email, is_superuser, is_staff, last_login zostawały jako pola FILTROWALNE. To blind oracle: `user.pbn_token startswith "…"` zwraca wiersze wtw. gdy prefiks pasuje → zalogowany redaktor (grupa „wprowadzanie danych") eksfiltruje cudzy (nawet superusera) token PBN znak-po-znaku → impersonacja w PBN. Także enumeracja superuserów przez `user.is_superuser = True`. Fix: override excluded() w BppQLSchema wykluczający get_user_model() z CAŁEGO schematu. Robione w excluded(), NIE atrybutem `exclude`, bo djangoql zabrania include+exclude naraz, a podklasa ustawia include. Cięcie na poziomie modelu łapie każdą ścieżkę do BppUser (nie tylko Autor.user) i przyszłe pola. API i publiczny /zapytanie/ NIE były podatne (allow-lista SEARCH_ALLOWLIST bez BppUser). Repro-test: bez fixu filtr po user.pbn_token przechodzi (oracle), po fixie jest odrzucany jako nieznane pole. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#4 (MEDIUM) z security review. SECRET_KEY z base.py fallbackował po cichu do jawnego sentinela z repo, gdy env DJANGO_BPP_SECRET_KEY nie był ustawiony — aplikacja startowała z publicznie znanym kluczem (ryzyko forgowania sesji i tokenów resetu hasła → przejęcie konta). Brak jakiegokolwiek hard-raise ani system-checka (jedyny z trójki SECRET_KEY/ALTCHA/DSPACE bez żadnego guardu). Fix: raise ImproperlyConfigured w production.py na placeholder. Świadomie w production.py, NIE system-checkiem: checki nie odpalają się na daphne/wsgi (get_asgi_application → django.setup bez checków), więc gate'owałyby tylko migrate; production.py jest importowany w KAŻDYM procesie (web/celery/migrate) → prawdziwe fail-closed. Predykat (secret_key_guard) to czysta funkcja: łapie pusty klucz, nieustawiony env (sentinel) i .env.docker bez zmian (ZMIEN...), a świadomie PRZEPUSZCZA build-owy dummy (collectstatic w obrazie), żeby nie wywalić builda. Dev/test na local.py — nietknięte. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tu XLSX #5 (MEDIUM) z security review. Import XLSX nie miał twardego limitu ROZMIARU pliku, a load_workbook czytał bez read_only → „gruby" formalnie-legalny plik (setki MB, po dekompresji pod progiem bomb-checka) wciągał cały skoroszyt do RAM i zabijał workera importu (OOM DoS; gated grupą „wprowadzanie danych"). Fix: - sprawdz_rozmiar_pliku() + PlikZaDuzyException: odrzuca plik > 100 MB PRZED load, obok istniejącego bomb-checka (oba importery: znajdz_naglowek, XLSImportFile). - load_workbook(read_only=True): strumieniowy odczyt (iteracja rows + cell.value) bez ładowania całego skoroszytu do RAM. Zweryfikowane, że cała ścieżka parsowania tylko czyta (brak ws.cell()/ws['A1']/.max_row/merged) i że realne pliki (w tym wieloarkuszowe) parsują się poprawnie. Przy okazji: test_zapisz_snapshot_* porównywał snapshot XLSX bajt-w-bajt — flaky, bo openpyxl stempluje docProps/core.xml aktualnym czasem przy każdym save(). Zmienione na porównanie ZAWARTOŚCI (nagłówki + wiersze) + sanity na komórkach. read_only spowalniał parsowanie o ułamek s i odsłaniał tego istniejącego flake'a częściej. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wynik równoległego security review (5 sub-agentów, attack-surface newralgiczny: auth · API+PII · DjangoQL · upload/import · injection/XSS/sekrety). Sześć potwierdzonych findingów, każdy z repro-testem (TDD: RED bez fixu → GREEN po).
Findingi
BppQLSchema, brak allow-listy) schodziła przez reverse-relacjęAutor.userdoBppUser—pbn_token(żywy token PBN),email,is_superuserbyły filtrowalne → blind oracle: redaktor eksfiltruje cudzy (nawet superusera) token PBN znak-po-znaku (user.pbn_token startswith "…"). API/web nie były podatne (allow-lista).excluded()wykluczającyget_user_model()z całego schematu (nie atrybutemexclude— konflikt zincludepodklasy).mark_safe-ował.escape()na wartościach przed interpolacją./api/v1/autor_jednostka/zwracał zatrudnienie (jednostka, daty, PK autora) także autorów ukrytych (pokazuj=False), obchodzącAutorViewSet.get_queryset()filtrujeautor__pokazuj=Truedla anonima./api/v1/zapytanie/*(ciężki multi-join DjangoQL) bez throttlingu → saturacja DB.throttle_classes = [SearchUserThrottle](endpointy wymagają logowania).SECRET_KEYfallbackował po cichu do placeholdera z repo, gdy env nieustawiony → produkcja startowała z publicznie znanym kluczem (forgowanie sesji/tokenów).raise ImproperlyConfiguredwproduction.py(importowany w każdym procesie — inaczej niż system-checki, które nie odpalają się na daphne). Build-owy dummy świadomie przepuszczony.load_workbookbezread_only→ „gruby" plik (setki MB) wciągał cały skoroszyt do RAM → OOM workera.sprawdz_rozmiar_pliku, 100 MB) przed load +read_only=True(strumieniowy odczyt). Zweryfikowane, że cała ścieżka parsowania tylko czyta.Przy okazji
Naprawiony pre-existing flaky test snapshotu #605: porównywał plik XLSX bajt-w-bajt, a openpyxl stempluje
docProps/core.xmlaktualnym czasem przy każdymsave(). Zmienione na porównanie zawartości (nagłówki + wiersze).Weryfikacja
import_common+import_pracownikow(1028),api_v1+przemapuj+ admin DjangoQL.ruff check+ruff formatczyste; 6 newsfragmentów (bugfix, PL).Poza zakresem
Drobne LOW z review (brak
AUTH_PASSWORD_VALIDATORSna ścieżce admin/CLI, TOCTOU w rate-limicie resetu hasła,FileExtensionValidatornaplik_xls, ekspozycja póluwagi/nie-public_) — do osobnego przejścia lub świadomego pominięcia.🤖 Generated with Claude Code