MIR for obm

[Chuck Short]

Binary package hint: obm

Request for OBM to be included into main, apart of the calendaring spec:

https://wiki.ubuntu.com/OBMMIR

If you have any questions please feel free to ask.

Regards
chuck

[Loïc Minier (lool)]

There were multiple CVEs in previous versions of OBM (typical web/PHP/input santizing issues: XSS, SQL injection), mostly affecting previous 1.x series, what steps are taken in the package to ensure the scripts aren't publicly exposed?

I would expect OBM to only be used by some key people in a company, from the company intranet, and with login/password; would I have to configure a SQL/PHP app, I might:
- restrict access to the service by IP address
- add some web server/Apache level filtering (e.g. require HTTP auth, or SSL client certs)

Grepping the source, I didn't see much escaping going on; nor parametrized SQL queries. I only found some stripslashes() and single quoting which is rather light.

I also looked for command executions from the PHP scripts, and found that the LDAP password will be updated by exec()ing $path/../auto/changePasswd.pl, is this mode of operation supported in the .deb packages? It seems the password is passed on the command line:
        $cmd = $cmd_ldap_passwd.$parameters.' --login '.$login.' --domain '.$domain_id.' --type '.$password_encryption." --passwd '".$new_pass."' --no-old";
this allows local users to see the new password.

I was a bit scared to see what other commands are defined:
// Automate commands
$cmd_halt = "sudo /sbin/halt";

As I understand it, if you install the package it will immediately be available from the web at /obm with login/password admin/admin. Shouldn't there be some debconf prompting to ask for an admin password before putting the service up?

[Martin Pitt (pitti)]

Thanks, Loic, for your review. The password passing and default password definitively need to be fixed. Also, why does a calendar application even need to know about "sudo" and shutting down the machine?

[Sylvain Garcia (sylvain-garcia)]

thanks for your review. So there is 3 bugs:

1 - the script $path/../auto/changePasswd.pl which use password on the command line.
  I agree with you and, I was discussed with all OBM dev team about this. But in Ubuntu, it isn't a problem because all perl program aren't installed. This part of OBM manage mail server (postfix, cyrus), ldap server,.... In Ubuntu version (and packging version) I don't build this because is very difficult for debian policy and conf files. In ubuntu there is just calendar/CRM/project..., just php/mysql.
So i was discussed with OBM team, but i think that it isn't a problem for the MIR.

2- OBM use "sudo /sbin/halt"; .... ;-)
   yes it's an old feature (OBM was born 8-9 years ago...), so it's unused feature for ubuntu package. I wil mak a patch.

3- login/password are already set.
  ok, I can use debconf to create first admin user, but i will add a README to describe how make a domain for OBM.

So I will resolve two bug: 2 and 3 before the first RC
Do you agree with me ? (in particular for hangePasswd.pl)

[Loïc Minier (lool)]

Sounds like a good plan; I didn't do a full security review though (I don't claim to have enough background for such a review), but in the light of the above issues, I think I will recommend that our security team takes a look before/just after main inclusion.

[Martin Pitt (pitti)]

1) Thanks for the explanation, so we can ignore that one.

2) If you mean you will just entirely disable this, fine :-)

3) debconf is okay, unless the server team plans to install this by default (then we cannot use debconf, it won't be shown). However, even in the latter case, people could still use "sudo dpkg-reconfigure", of course (or edit a conffile, or run "sudo obm-password" or so).

[Nick Barcet (nijaba)]

Regarding 3), no plan to install OBM by default, so I think we are safe.

[Sylvain Garcia (sylvain-garcia)]

I have resolve this bugs:

1) nothing
2) I make a patch to disable this ;-)
3) Now debconf ask login and password for OBM global admin, so i upload new sql file to remove all default users.

Do you have other suggestion to main inclusion?

Tonio had upload OBM 2.1.10-0ubuntu2 version, thank you Tonio ;-)

[Loïc Minier (lool)]

Subscribed Ubuntu Security: folks, obm is a not so small PHP app; it's programming model seems coherent, but I think it'd be best if you could take a look before we promote this to main.

[Martin Pitt (pitti)]

waiting for input of ubuntu-security. Kees, with being the intersection of ubuntu-mir and ubuntu-security, could you please take a look at this? Thanks!

[Kees Cook (kees)]

I've got to reject this. There has been absolutely no attempt to protect this application from SQL injection.

For example:
function check_privacy($module, $table, $action, $id='', $p_uid='') {
...
  $query = "SELECT $field_pri, $field_uc FROM $table WHERE $field_id = '$id'";
  $obm_q = new DB_OBM;
  $obm_q->query($query);

...
$params = get_list_params();
...
if (! check_privacy($module, "List", $action, $params["list_id"], $obm["uid"])) {

where get_list_params is virtually untouched $_POST values.

Before this gets further review, all database use should be correctly parameterized. And it's not a short list, I'm afraid:
$ fgrep -Ri -- '->query($query);' . | wc -l
977

[Sylvain Garcia (sylvain-garcia)]

Hi kees, sorry for this delay.... :-(

I agree with, you about reject of OBM.
OBM is an old project which had make good code and bad code.
I had discuss with th principal developer of php. He work on background in order to remove, and improve security on this.

I wouldlike say that obm require "magic quotes", so sql injection is limited. I know, is bad to use this but there is an history...

So, thank you any way, about this discuss on MIR. OBM is project which increase, and i wouldlike even though in the futur build a thing between Ubuntu and OBM

Tx Nick, Loïc, Martin, Jamie, and me :P