Thomas Goirand <[email protected]> writes: > Now, let me take a step back and try to comment on the big picture. > > To make things clear: I do understand that these last MySQL and shell > insertion are really serious issues with grave consequences, and I > agree that some parts of the code is sloppy to say the least. I am the > one to blame for sure, for not looking enough at contributions and > re-factoring code enough. I really don't want you guys to think that > I'm not taking it seriously. I'd like however that you understand that > a lot of work has been recently (over let's say the last 2 years), to > re-factor the code. And that this work will continue.
I do not think any single of the reported bug lead to this removal request, but when I did look at dtc's code I did find several systematic problems all over the code (not only in some parts) that resulted in all the security bugs I reported so far: * No parameter binding: all SQL queries are build using string manipulation; most parameters come directly from $_REQUEST, escaping (via mysql_real_escape_string) is only done in some places. * No scheme used to make sure only sanitized variables are ever used. Together with the first point this makes SQL injections very likely. * Checking variables and using them often happens at totally different places (often different files). This makes it even harder to make sure they are always safe to use. * Related to the last point, control flow is a mess. Most seems to be sphagetti code, there is a lot of duplication, many global variables, etc. * Variables used in HTML output are not escaped either. I did not yet look at this in detail. * exec(...) is used. It's not really needed for appending lines to a given file, and most other uses do at least not require a shell to be involved either. * No priviledge separation: everything -- including apache -- runs as the user "dtc" which also owns config files for apache, bind and others. This probably makes this user root-equivalent. All of this together make it in my opinion impossible to write a secure program, many parts seem to do the exact opposite of good practices. The rest of this mail are some comments on specific bugs. Feel free to skip them, they do not matter too much to this discussion, but I think demonstrate some of the problems above: > #611680 isn't relevant and has no impact, as written on the BTS. It has some impact as it makes the code more brittle: when a small breach is found, it is easy to escalate it higher. > #566654 I believe, isn't more dangerous than the /etc/mysql/debian.cnf. > So unless /etc/mysql/debian.cnf is removed from Debian, fixing #566654 > is useless, IMHO. dtc is accessible remotely, the system account for mysql should only be accessible locally. This means read access to files (or backups) gives remote access; to make use of the mysql system account, you would already need to have some kind of local access. In any case I don't think other programs being not perfect should not invalidate this bug (I would prefer mysql would not need the system account, but that's a different topic). [ About #637477 and #637487: ] > I have already fixed what Ansgar reported, and I am currently working > on other fixes. [...] These patches[1] look more like a bandaid than a proper solution. I hope you do not see them as fixed :/ [1] <http://git.gplhost.com/gitweb/?p=dtc.git;a=commitdiff;h=01c4eea40736b89cb396b0596f2808c8f64b8729> > [#637482] (eg: the issue is the hostname of the server not being > configured correctly, and "hostname --domain" not replying anything, > which isn't a DTC issue per say, but more an administrator issue). This is minor, but dtc did not complain when I gave an empty domain name. It should give a proper error message when it does expect one. Regards, Ansgar -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
