VNG-Realisatie / Haal-Centraal-common

Project repository tbv de ontwikkeling van de over alle Haal Centraal API specificaties gedeelde specificaties
Other
2 stars 7 forks source link

Foutmelding toevoegen voor foutieve parameter waarden #99

Open strijm opened 3 years ago

strijm commented 3 years ago

Als dienst beheerder willen we de foutafhandeling.feature uitbreiden met een foutmelding voor de situatie dat een parameter (een reeks van) karakters bevat die niet is toegestaan.

| validatie | voorbeeld reason | code | | paramWaarde | Parameter bevat niet toegestane karakters | notAllowedCharacter |

Dit is ten behoeve encoding / escaping voor velden waar geen pattern validatie op zit. De overige foutmelding zijn niet geschikt voor deze situatie.

fsamwel commented 3 years ago

Ik neem deze over. @strijm ik heb wel een punt aan het einde van de reason gezet (consistentie met andere meldingen).

Kan er in de melding welke karakter(s) niet zijn toegestaan? Anders is het voor de gebruiker onduidelijk wat er precies niet correct is.

"Parameter bevat niet toegestane karakters. De volgende karakters zijn niet toegestaan: {notAllowedCharactersList}."

Bijvoorbeeld: "Parameter bevat niet toegestane karakters. De volgende karakters zijn niet toegestaan: " < > % { } | \ ^ `."

Alternatief is om in de melding al aan te geven dat het om onveilige karakters gaat (ik neem aan dat een developer dan wel weet om welke karakters het gaat). | validatie | voorbeeld reason | code | | injectie, XXS, XSRF | Parameter bevat niet toegestane karakters die onveilig kunnen zijn. | unsaveChars |

fsamwel commented 3 years ago

@MelvLee @JohanBoer Wat vinden jullie de beste melding?

Of kunnen we beter een pattern toevoegen aan alle parameters om de onveilige karakters uit te sluiten?

MelvLee commented 3 years ago

ik vraag me af of het mogelijk is om op een generieke manier aan te geven er karakters unsafe zijn. De % teken wordt gebruikt om karakters te escapen. Als je zou willen zoeken op bijv. 'laan van meerdervoort' dan zou dat niet lukken omdat je dan 'laan%20van%20meerdervoort' als query krijgt. Je zou dan de '+' moeten gebruiken om de spatie te escapen. Als ik in Google op de volgende tekst zoek 'dit % > is | een test', dan zie ik het volgende als query parameter: 'dit+%25+>+is+|+een+test'. En als ik het volgende meegeef als input aan een zoek parameter 'schoolstraat;drop table adres;' of 'jansen or 1=1', dan is de kans groter dat ik sql heb geinject terwijl er geen unsafe karakters in staan

JohanBoer commented 3 years ago

De conclusie van Melvin's opmerking is volgens mij dat je het met een pattern niet helemaal waterdicht krijgt en dat je sommige karakter (%) gewoon nodig hebt. Neemt niet weg dat een pattern toevoegen in ieder geval de niet-gewenste unsafe karakters uitsluit de boel iets minder onveilig maakt, of zie ik dat verkeerd ?

Dan is er nog steeds additionele validatie nodig die dan weer om een eigen foutmelding vraagt. Is dat dan gewenst ?

Ergo, met het toevoegen van een pattern kan je misschien een paar unsafe karakters uitsluiten en daar een inhoudelijke melding op geven, maar de melding zoals Mark die voorstelt blijft ook nodig. @strijm Je kan in de reason toch ook de karakters retourneren die de fout hebben veroorzaakt ? Of is dat not done ?

strijm commented 3 years ago

Bij voorkeur geef je zo min mogelijk informatie die een kwaadwillende op het spoor kan zetten van evt. vulnerabilities. Dus noemen dat een parameter een XSS, XSRF of SQL injectie validatie niet heeft doorstaan, lijkt me idd. not done, evenals het benoemen welke karakters niet zijn toegestaan. In dit geval zo neutraal mogelijk aangeven dat bepaalde karakters niet zijn toegestaan. Ik ben ook van mening dat het controleren met een pattern sommige maar niet alle foutsituaties zal oplossen. Aan het voorbeeld van Google in het bericht van Melvin is te zien dat security encoding is toegepast, m.i. is dat de beste methode.