Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a new list with a ` in the password will result in a crashing cronjob every minute #1

Open
lelutin opened this issue Apr 13, 2016 · 4 comments

Comments

@lelutin
Copy link

lelutin commented Apr 13, 2016

The password field is either too permissive with its contents or the way the password is being set is problematic. in any case, when a new mailing list is created and a backtick is present in the password, the update_mailman.sh cronjob will crash on every run.

this is caused by the way "newlist" is called here:

su - list -c "/usr/lib/mailman/bin/newlist -q \"$list@$domain\" \"$owner\" \"$password\""

the backtick gets interpreted and sh complains about a syntax error.

I don't know yet if that might have security implications too: I haven't tested whether shell injection is possible through the password, but I surmise it probably is.

my suggestion would be to make alternc reject passwords that contain a backtick.

note: this might be applicable to double quotes too

@fser
Copy link

fser commented Apr 13, 2016

printf could be a solution. Using %q format string prints the escaped value for that argument. For instance:

$ printf "%q\n" 'su id || echo plop'
su\ id\ \|\|\ echo\ plop

@lelutin
Copy link
Author

lelutin commented Apr 23, 2016

@fser je viens de tester la solution que tu propose et ça fonctionne bien pour les "backticks" mais ça a le facheux résultat de transformer tout le reste en ajoutant un backslash devant.

par exemple un mot de passe "allo toi" doit être écrit "allo\ toi" dans mailman pour pouvoir entrer. donc le mot de passe final ne correspond pas à ce que les utilisateurs ont spécifié.

donc je crois que la seule solution c'est de refuser les backticks dans les formulaires de création de liste et changement de mot de passe de liste. malheureusement ça ne nous protège pas contre le SQL injection.
c'est probablement le cas pour toutes les colonnes dans la table "mailman" également: si qqun arrive à modifier la valeur d'une colonne pour y insérer des backticks et changer l'action pour que le mot de passe soit changé par exemple, le serveur va se faire défoncer.

par ailleurs j'ai testé mon hypothèse par rapport à la sécurité et on a bel et bien un problème d'injection de commandes!
si qqun entre par exemple comme mot de passe de liste:

`nc -l 65432 | bash`

la cronjob va ouvrir le port 65432 et tout ce qui y sera reçu sera donné à bash pour exécution (donc via le champs de mot de passe des mailing lists on peut facilement ouvrir une backdoor)

@lelutin
Copy link
Author

lelutin commented May 5, 2016

fser m'a proposé une nouvelle version de patch (je copie du lien qui m'avait été donné parce qu'il est vraiment lent et je ne sais pas si le contenu sera disponible dans le futur):

diff --git a/src/update_mailman.sh b/src/update_mailman.sh
index 98ad6a0..c70626d 100755
--- a/src/update_mailman.sh
+++ b/src/update_mailman.sh
@@ -40,8 +40,9 @@ mysql_query "SELECT id,list, name, domain, owner, password FROM mailman WHERE ma
     then
     mysql_query "UPDATE mailman SET password='', mailman_result='This list already exist', mailman_action='OK' WHERE id='$id';"
     else
-      # Create the list : 
-      su - list -c "/usr/lib/mailman/bin/newlist -q \"$list@$domain\" \"$owner\" \"$password\""
+      # Create the list :
+      CMD=$(printf "\"/usr/lib/mailman/bin/newlist -q '%s@%s' '%s' '%s'\"" "$list" "$domain" "$owner" "$password")
+      su - list -c "$CMD"
       if [ "$?" -eq "0" ]
       then
         mysql_query "UPDATE mailman SET password='', mailman_result='', mailman_action='OK' WHERE id='$id';"

la patch finale devra inclure la même modification sur l'action de changement de mot de passe.

je vais tester ce changement maintenant

@lelutin
Copy link
Author

lelutin commented May 5, 2016

la modification n'a pas fonctionné: j'obtenais l'erreur suivante dans le retour email de la cronjob:

-su: /usr/lib/mailman/bin/newlist -q 'navet@compte123.koumbit.org' 'navet@koumbit.org' '': Aucun fichier ou dossier de ce type

pour tenter de faire fonctionner le script j'ai retiré les " à l'intérieur du printf, mais alors la commande de création de liste a réussie mais la commande insérée dans le mot de passe a tout de même été interprétée.

donc retour à la case départ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants