Hooks can lead to arbitrary code execution

Bonjour,

Pouvez-vous envoyer les arguments au script définit en pre-check entre guillement svp ?

Ex : check-password.pl "motdepassedelamortquitue"

(from redmine: issue id 4875, created on 2016-06-13, closed on 2016-06-23)

• Relations:
  • relates #4898
• Changesets:
  • Revision e6ff8973 on 2016-06-23T02:41:19.000Z:

Fixes #4875 escaping shell arguments when calling hooks

• Revision 2f3aa001 on 2016-06-23T02:42:13.000Z:

Fixes #4875 escaping shell arguments when calling hooks

• Custom Fields:
  • Bug in version: 1.0.12

hello,

i'am not sure about this, lets talk to the developper :)

Cheers

(from redmine: written on 2016-06-16)

By bmortier on 2017-09-02T15:30:00 (imported from GitLab)

For me this should not be changed as it would break existing hooks. Also the same behavior is used for placeholders in hooks and templates and we obviously do not want to have templates full of "" in each field so I’m not sure how easy it would be to implement.

And, in some cases, it can be useful to be able to quote where the user wants, for instance: @echo "user %cn%(%uid%) has been modified" > /tmp/somelog.txt@

(from redmine: written on 2016-06-22)

By Côme Chilliet on 2017-09-02T15:30:00 (imported from GitLab)

Ok, so i'm facing a problem

My script check for the "force" of password, each password can contains ';' or '>' or '#' and '!'. This cause lots of trouble when the script is executed and in fact somebody can play with my server... imagine if the password entered is "a; touch /tmp/t"

/usr/share/fusiondirectory/script.pl a; touch /tmp/t

Lots of fun ...

(from redmine: written on 2016-06-22)

By amichon on 2017-09-02T15:30:00 (imported from GitLab)

In fact, add " will not solve the problem

(from redmine: written on 2016-06-22)

By amichon on 2017-09-02T15:30:00 (imported from GitLab)

Indeed this is a big problem, even if the user uses quotes, if the attacker enter a closing quote he can execute code. We should use http://php.net/manual/en/function.escapeshellarg.php on placeholders final values before running any hook. But this means people must not put quotes in their hooks, and they won’t be able to do things like «@echo "%value1% %value2%" > /tmp/somefile@» for instance. But I don’t see any way to prevent attacks and still allow this kind of thing as the escaping is not the same one depending of type of quotes used. We could add escaping only as a modifier but most people would forget to put it and expose themselves. Maybe we can do the opposite and have a modifier to prevent escaping in special cases.

(from redmine: written on 2016-06-22)

By Côme Chilliet on 2017-09-02T15:30:01 (imported from GitLab)

hello,

yes i wote for escaping always and have a special modifier if people want to be exposed :)

And of course i always advised people not to put quote inside the command in the hooks section, better put the quote inside the script

Cheers

(from redmine: written on 2016-06-22)

By bmortier on 2017-09-02T15:30:01 (imported from GitLab)

Patching : ok

I have done two or three tests, all are working in the right way :

• a; touch /tmp/z; : ok
• true password : ok
• false password : ok (false means using toto as password)
• random letter and special character : ok

(from redmine: written on 2016-06-23)

By amichon on 2017-09-02T15:30:02 (imported from GitLab)

The patch is now using in production environnement.

(from redmine: written on 2016-06-23)

By amichon on 2017-09-02T15:30:02 (imported from GitLab)

Close issue

(from redmine: written on 2016-06-23)

By Jonathan Swaelens on 2017-09-02T15:30:02 (imported from GitLab)

closed

By Jonathan Swaelens on 2017-09-02T15:30:03 (imported from GitLab)

added Security label

By bmortier on 2018-09-28T19:38:53 (imported from GitLab)

removed Bugs Security labels

added Security fusiondirectory-core labels

added FSA-0010 label

added FSA-0009 label and removed FSA-0010 label