dolibarr-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Dolibarr-dev] Vulnerabilities


From: Philip Lehmann-Böhm
Subject: Re: [Dolibarr-dev] Vulnerabilities
Date: Tue, 05 Nov 2013 22:03:38 +0100
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0

Hi,

that's a reason, yes. It still needs to be really prominent. Either
don't store passwords as default or let the user make the decision
directly in the setup.
Salting in 3.5 is good. MD5 not, sorry. :) Assume it to be cracked,
there are fully working rainbow tables. Maybe 3.5 is a possibility to
switch to bcrypt, sha2 or something similar. A good summary is this posting:
http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords

Cheers
Philip

Am 04.11.2013 10:48, schrieb Destailleur Laurent:
> It is an option because some users need to interface dolibarr with some
> external system that needs authentication with clear password. In such
> case, api or dolibarr addon/modules et batch needs to find password (an
> example is module ldap).
> When option is on, password is hashed with a md5 password.
> 
> With version 3.5, parameter "MAIN_SECURITY_SALT" will allow to choose
> value for the salt.
> 
> In most cases, when we don't have the escape into the sql function, it
> is because the data is not a data coming from a user input but a data
> computed by dolibarr code because calling the request.
> For example conf->entity is a data always defined by dolibarr and does
> not depends from user input.
> But you are right, it would be safer to always escape parameter.
> 
> This is a good remind for developper.
> 
> 
> 
> 2013/11/3 Philip Lehmann-Böhm <address@hidden
> <mailto:address@hidden>>
> 
>     Hi, thank you for the answer.
>     Ok,
>     why is this an option? This is just not optional. Thanks for pointing me
>     to it, I wouldn't have found it. :) The whole column "pass" should just
>     be removed.
>     How is the password crypted? I assume a salted and hashed password?
> 
>     How comes that you use an own function to escape query parameters and
>     not the buildin ones?
>     Take for example this one: admin/facture.php
> 
>     line 161: $sql.= " WHERE nom = '".$db->escape($value)."'";
> 
>     line 194:     $sql.= " VALUES ('".$value."', '".$type."',
>     ".$conf->entity.", ";
> 
>     $value is the same variable, one time escaped, one time not. Maybe this
>     is dangerous, maybe not. If you'd use prepared statements and not string
>     concatenation, this would not have slipped through.
> 
>     No offense meant, this is just important. :)
> 
>     Am 03.11.2013 22 <tel:03.11.2013%2022>:11, schrieb Destailleur Laurent:
>     > There is already an option to have password crypted into database
>     (menu
>     > security - encrypt password)
>     >
>     > Also you suggest to use a method to sanitized sql request parameters.
>     > Using such a function to clean sql parameters is already done. The
>     > function is called db->escape (a method of mysql.class.php). Port
>     to use
>     > it step by step was started few years ago.
>     >
>     > In a future the continuous integration platform should also be able to
>     > cry when escaping parameters will be forgotten.
>     >
>     >
>     >
>     > 2013/11/3 Philip Lehmann-Böhm <address@hidden
>     <mailto:address@hidden>
>     > <mailto:address@hidden <mailto:address@hidden>>>
>     >
>     >     Hi,
>     >
>     >     (sorry, I don't know how to reply directly to the existing thread:
>     >    
>     http://lists.nongnu.org/archive/html/dolibarr-dev/2013-10/msg00003.html
>     >     )
>     >
>     >     This just blew my mind a bit. In this topic, especialy the
>     denial of
>     >     starting to use parametrized queries.
>     >     And that the password is stored in plain text in the database is a
>     >     no go.
>     >
>     >     And the statement, that everything of the quoted website has
>     been fixed
>     >     is not true. I run a freshly installed Dolibarr 3.4.1 and the
>     passwords
>     >     are indeed available in plain text!
>     >
>     >     I'm willing to help here and this is what I propose:
>     >     - Are there plans to drop the plain password column? Has this
>     already
>     >     happened in the next version? This goes to much in the core of
>     Dolibarr,
>     >     so I won't be able to patch this in a meaningful timespan.
>     >
>     >     - Not using prepared statements is a no go as well. I'd add
>     support for
>     >     them in the mysql.class.php (not familiar with the others) with a
>     >     function like this:
>     >     function parametrizedQuery($query, $params,
>     >     $usesavepoint=0,$type='auto')
>     >     And then start to port the code to use it step by step and
>     making some
>     >     pull requests.
>     >
>     >     What do you think? Would this be a way to go?
>     >
>     >     Best Regards
>     >     Philip
>     >
>     >     _______________________________________________
>     >     Dolibarr-dev mailing list
>     >     address@hidden <mailto:address@hidden>
>     <mailto:address@hidden <mailto:address@hidden>>
>     >     https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
>     >
>     >
>     >
>     >
>     > _______________________________________________
>     > Dolibarr-dev mailing list
>     > address@hidden <mailto:address@hidden>
>     > https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
>     >
> 
>     _______________________________________________
>     Dolibarr-dev mailing list
>     address@hidden <mailto:address@hidden>
>     https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
> 
> 
> 
> 
> _______________________________________________
> Dolibarr-dev mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]