Hotfix/prevent ldap injection #492
No reviewers
Labels
No labels
devtype -- backend
devtype -- docs
devtype -- frontend
devtype -- user interface
difficulty -- easy
difficulty -- hard
difficulty -- normal
Doing
domain -- bda
domain -- bds
domain -- cof
domain -- core
domain -- kfet
Good first issue
priority -- high
priority -- low
priority -- medium
priority -- staff-wanted
status -- development
status -- discussion
status -- need review
status -- production
status -- ready to merge
status -- todo
To Do
type -- bug
type -- hygiene
type -- improvement
type -- new feature
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: DGNum/gestioCOF#492
Loading…
Reference in a new issue
No description provided.
Delete branch "hotfix/prevent_ldap_injection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Prevent LDAP injections in the autocompletion process by restricting the query to the alphanumeric words.
We let the users know about it by adding a help-text above each autocompletion input.
I can push this code to https://dev.cof.ens.fr/gestion upon request to allow the reviewers to test it on the actual LDAP database.
Fixes #150
Je me trompe peut-être mais j'ai l'impression que le code dit "les caractères non alphanumérique sont ignorés" alors que ton message aux utilisateurs dit "les mots contenant des caractères non alphanumérique sont ignorés". Non ?
Non ce sont bien les mots qui sont ignorés : cette ligne signifie que si je vais la recherche
erkan narm foo*
, on aurabits = ["erkan", "narm", "foo*"]
et donc la requête sur le LDAP ne se fera qu'avec les deux premiers motsAlso, je ne suis pas complètement satisfait de ma solution. L'idéal serait de sanitize la requête au lieu de la vider mais je n'ai pas trouvé comment faire
Comme ça :
re.sub(r'\W+', '', bit)
?Non je veux parler d'un vrai sanitize (comme
prepare
pour les requêtes SQL), ça existe déjà dans la lib PHP pour ldap. Typiquement il faudrait échapper les parenthèses, les*
, etc… Mais l'implémenter nous même ne me semble pas être une bonne idée carTa solution comme la mienne évite le problème. Cela étant dit, c'est sûrement mieux effectivement de supprimer les caractères spéciaux plutôt que d'enlever des mots entiers.
Ok ok :) tu peux passer cette branche sur dev que je finisse de review ? :)
Pourquoi
''.join( [ gnagnagna for x in truc ] )
et pas''.join( gnagnagna for x in truc )
?Parce que je n'y pense jamais. Bien vu
added 1 commit
4899bba1
- Replace list comprehensions by iterators in dalCompare with previous version
@narmanli done :)
added 1 commit
dc50a23b
- Escape the content of the regex inhighligh_text
Compare with previous version
added 1 commit
74f0b63e
- Change ldap query behaviour if multiple inputsCompare with previous version
typo
Il faut aussi changer ici en
(&)
car le dernier commit modifie ce que donne une requête vide.Idem
added 1 commit
0aed9756
- Fix the empty-query test in autocompleteCompare with previous version
Nonobstant la typo, je suis OK pour merge =)
added 1 commit
59f57793
- typoCompare with previous version
mentioned in commit
ae38b5d1e7
merged