Re: dbmail SQL structure, SCHEMA support, SQL injection

On 29-05-14 01:20, Jean-Michel Pouré - GOOZE wrote:
> Thank you for explanations.
>
>> Lightweight copying of messages, ability to link messages to multiple
>> mailboxes.
> [...]
>> JOINS are unavoidable in a highly normalized schema. If you know a
>> better way to retrieve the same information, please share.
>
> OK, I understand the logic now.
>
> IMHO, it should not be allowed to link a single message to multiple
> mailboxes. Because multiple mailboxes are read by different people. This
> opens a possible security breach.

Sigh. You are giving opinions based on lack of information, talking 
about security without providing the actual threat vector. There is no 
security threat involved when a single rfc2822 message (a physmessage) 
is references by multiple mailboxes owned by multiple users. Any 
information that is pertinent to a mailbox is only accessible by the 
owner of that mailbox - unless the mailbox is shared via an ACL 
configuration.

>> Security issues from JOINs?? what are you talking about?
>
> This one in imapd.c:
> p_string_printf(query, "SELECT m.message_idnr, n.headername,
> v.headervalue%s "
> "FROM %sheader h"
> "LEFT JOIN %smessages m ON h.physmessage_id=m.physmessage_id"
> "LEFT JOIN %sheadername n ON h.headername_id=n.id"
> "LEFT JOIN %sheadervalue v ON h.headervalue_id=v.id"
> "WHERE m.mailbox_idnr = %" PRIu64 ""
> "AND m.message_idnr %s"
> "AND n.headername %s IN ('%s')"
> "ORDER BY message_idnr, seq",
> not?"":fieldorder->str,
> DBPFX, DBPFX, DBPFX, DBPFX,
> self->mailbox->id, p_string_str(range),
> not?"NOT":"", bodyfetch->hdrnames);
>
> IF(?) someone is able to inject m.mailbox_idnr value using a modified
> imap client, this query allows to read any emails in the database.

Of course. Earlier versions of dbmail added the owner_idnr into the mix 
for that reason. But the mailbox_idnr in this case is trusted, making 
such an additional join redundant.

The mailbox idnr value does not originate from the mail client. A 
mailclient opens a mailbox by name (x select INBOX), and the dbmail imap 
code then determines the mailbox idnr from there. The list of 
headernames being matched against here is open for auditing. You might 
be able to inject a colon there, but no spaces. Seems difficult to 
construct a malicious query without spaces...

>
> An SQL injection can happen when there is a security breach somewhere
> else.

Sure. So? I did a full Coverity fixup earlier this year against the 
master branch. Nothing major was uncovered. Lots of small cleanup. And 
it would require a serious buffer overflow, stack trash or a similar 
attack to inject malicious SQL, as far as I can tell. I'm not saying it 
can't be done. But general assertions that DBMail is at risk without any 
kind of proof, is plain FUD.

>
>>> 2) Privilege separation
>>>
>>> I read the post on the WIKI with interest and would like to add this
>>> information:
>>
>> what post is that?
>
> http://www.dbmail.org/dokuwiki/doku.php/privsep

Ah. I don't know who wrote that. The implications of such a change are 
being underestimated, and benefits are vague and unclear - even in 
security terms.

> Why not change this and have:
> * one DB user
> * and one SCHEMA
> => for each IMAP account?
>
>>> Using schemas, it may be possible to create as many database structures
>>> using an imap identifier ([email protected]).
>>
>> Ok. Sounds cool. But can you still share some tables? Like the main
>> message store?
>
> Again, I think sharing information between users is a security breach.

Shared mailboxes are a feature. And the mailstore is optimized for 
de-duplicating content. Kind of like throwing out the baby with the 
bath-water (a dutch saying, maybe?).

> The main reasons why servers are getting hacked are (1) dns forgery (2)
> email systems. Penetration of email systems are very painful as they
> reveal secrets.

Of course. Opening your mailbox and seeing someone else's email is not 
pretty.

>
>>> Could you point me to any filter against database injection in the
>>> source code. Where is it located in source code?
>
> It is very likely that some SQL injection is possible in the code. I did
> not test or looked deeply, but since you dont' filter SQL statements AND
> share tables between users, it is ***VERY*** likely.

There is *nothing* to filter! End-users can not inject SQL, afaik. All 
queries are either constructed as prepared statements, or constructed 
from content from trusted (internal) sources.

> I am not sure whether mySQL supports schemas, for sure it can be
> emulated using table prefix and proper ownership. By the way, I am not
> very interested in MySQL, as the developer community is dead and MySQL
> is non-standard and slow and low-featured (but always boasting to be the
> best).

MariaDB seems pretty alive and kicking. And the installed base of MySQL 
is very impressive.

For the truly paranoid, it's trivial to run separate databases per user. 
Just set up a database per user, use LDAP for central authentication, 
and use some proxy like Perdition or nginx to connect to the right 
database. You'll have to add user-based transports in SMTP, but once you 
do that, you're all set. Of course, scalability sucks in such a setup. 
But what the heck.


-- 
________________________________________________________________
Paul J Stevens       pjstevns @ gmail, twitter, github, linkedin
            www.nfg.nl/[email protected]/+31.85.877.99.97