DGNum/gestioCOF
15
2
Fork 1
Code Issues 78 Pull requests 14 Projects Releases 12 Packages Wiki Activity

Hotfix/prevent ldap injection #492

Merged
mpepin merged 0 commits from hotfix/prevent_ldap_injection into master 2017-03-20 23:07:04 +01:00
Conversation 20 Commits - Files changed -
mpepin commented 2017-03-17 00:25:15 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

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

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
narmanli commented 2017-03-18 10:43:53 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

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 ?

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 ?
mpepin commented 2017-03-18 12:53:19 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Non ce sont bien les mots qui sont ignorés : cette ligne signifie que si je vais la recherche erkan narm foo*, on aura bits = ["erkan", "narm", "foo*"] et donc la requête sur le LDAP ne se fera qu'avec les deux premiers mots

Non ce sont bien les mots qui sont ignorés : cette [ligne](https://git.eleves.ens.fr/cof-geek/gestioCOF/blob/master/gestioncof/autocomplete.py#L31) signifie que si je vais la recherche `erkan narm foo*`, on aura `bits = ["erkan", "narm", "foo*"]` et donc la requête sur le LDAP ne se fera qu'avec les deux premiers mots
mpepin commented 2017-03-18 12:54:14 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Also, 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

Also, 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
narmanli commented 2017-03-18 15:14:13 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Comme ça : re.sub(r'\W+', '', bit) ?

Comme ça : `re.sub(r'\W+', '', bit)` ?
mpepin commented 2017-03-18 21:13:45 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

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 car

  1. On connait mal le protocole LDAP (at least me)
  2. C'est une responsabilité de maintenir un truc comme ça

Ta 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.

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 car 1. On connait mal le protocole LDAP (at least me) 2. C'est une responsabilité de maintenir un truc comme ça Ta 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.
narmanli commented 2017-03-19 14:50:52 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Ok ok :) tu peux passer cette branche sur dev que je finisse de review ? :)

Ok ok :) tu peux passer cette branche sur dev que je finisse de review ? :)
manet commented 2017-03-19 14:55:38 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Pourquoi ''.join( [ gnagnagna for x in truc ] ) et pas ''.join( gnagnagna for x in truc ) ?

Pourquoi `''.join( [ gnagnagna for x in truc ] )` et pas `''.join( gnagnagna for x in truc )` ?
mpepin commented 2017-03-19 15:03:04 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Parce que je n'y pense jamais. Bien vu

Parce que je n'y pense jamais. Bien vu
mpepin commented 2017-03-19 15:06:15 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 4899bba1 - Replace list comprehensions by iterators in dal

Compare with previous version

added 1 commit * 4899bba1 - Replace list comprehensions by iterators in dal [Compare with previous version](https://git.eleves.ens.fr/cof-geek/gestioCOF/merge_requests/188/diffs?diff_id=244&start_sha=55b67f38c8a4e61ddf7e51a4e851ed23e2a4204b)
mpepin commented 2017-03-19 15:13:56 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

@narmanli done :)

@narmanli done :)
mpepin commented 2017-03-19 15:50:39 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • dc50a23b - Escape the content of the regex in highligh_text

Compare with previous version

added 1 commit * dc50a23b - Escape the content of the regex in `highligh_text` [Compare with previous version](https://git.eleves.ens.fr/cof-geek/gestioCOF/merge_requests/188/diffs?diff_id=245&start_sha=4899bba1f78168b78ad6e353eb2d6f08ef87577e)
mpepin commented 2017-03-19 16:10:19 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 74f0b63e - Change ldap query behaviour if multiple inputs

Compare with previous version

added 1 commit * 74f0b63e - Change ldap query behaviour if multiple inputs [Compare with previous version](https://git.eleves.ens.fr/cof-geek/gestioCOF/merge_requests/188/diffs?diff_id=246&start_sha=dc50a23b1045fcddd3dd486c7d43f9402ccb4b6c)
lstephan commented 2017-03-20 05:06:26 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

typo

typo
lstephan commented 2017-03-20 05:09:06 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Il faut aussi changer ici en (&) car le dernier commit modifie ce que donne une requête vide.

Il faut aussi changer ici en `(&)` car le dernier commit modifie ce que donne une requête vide.
lstephan commented 2017-03-20 05:09:38 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Idem

Idem
mpepin commented 2017-03-20 09:17:17 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 0aed9756 - Fix the empty-query test in autocomplete

Compare with previous version

added 1 commit * 0aed9756 - Fix the empty-query test in autocomplete [Compare with previous version](https://git.eleves.ens.fr/cof-geek/gestioCOF/merge_requests/188/diffs?diff_id=254&start_sha=74f0b63e9641504b25c1c865a12fdee6d3fb0ce1)
lstephan commented 2017-03-20 16:34:23 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

Nonobstant la typo, je suis OK pour merge =)

Nonobstant la typo, je suis OK pour merge =)
mpepin commented 2017-03-20 23:06:03 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

Compare with previous version

added 1 commit * 59f57793 - typo [Compare with previous version](https://git.eleves.ens.fr/cof-geek/gestioCOF/merge_requests/188/diffs?diff_id=255&start_sha=0aed975615f9f5869223742f2c11287b13a02129)
mpepin commented 2017-03-20 23:07:02 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

mentioned in commit ae38b5d1e7

mentioned in commit ae38b5d1e7f7697805034ea99048235a6844bd04
mpepin commented 2017-03-20 23:07:04 +01:00 (Migrated from git.eleves.ens.fr)
Copy link

merged

merged
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
devtype -- backend

Au programme : du Python, du Django...

  
devtype -- docs

   
devtype -- frontend

   
devtype -- user interface

Au programme : templating, JS, CSS...

  
difficulty -- easy

   
difficulty -- hard

Difficile et/ou risqué

  
difficulty -- normal

   
Doing

   
domain -- bda

Concerne l'application BdA

  
domain -- bds

   
domain -- cof

Concerne l'application COF

  
domain -- core

Dépendances externes, CI, linting...

  
domain -- kfet

Concerne l'application K-Fêt

  
Good first issue

   
priority -- high

Urgent quoi

  
priority -- low

   
priority -- medium

   
priority -- staff-wanted

L'équipe dirigeante du domaine en question souhaite cette fonctionnalité.

  
status -- development

En cours de développement

  
status -- discussion

Avant de coder, c'est bien de discuter

  
status -- need review

En attente de code review

  
status -- production

L'ajout/la modification est présente en production

  
status -- ready to merge

Plus qu'à merger !

  
status -- todo

Le développement devrait bientôt commencer

  
To Do

   
type -- bug

Signalement ou correction de bug

  
type -- hygiene

Tout ce qui amène à du code plus propre et un repo mieux organisé.

  
type -- improvement

Ajout/Demande d'évolution mineure d'une fonctionnalité existante

  
type -- new feature

Ajout/Demande d'une nouvelle fonctionnalité

  
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

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
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: DGNum/gestioCOF#492
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?