Full disclosure for SA45649, SQL Injection in LedgerSMB and SQL-Ledger

[Chris Travers <chris@xxxxxxxxxxxxxxxx>]

Affects versions:
SQL-Ledger 2.8.33 and lower
LedgerSMB 1.2.24 and lower.

Both programs have vendor fixes available in the form of new, patched
versions.  These have been out for over a week with appropriate
advisories, with users having time to upgrade.

Files affected:  LedgerSMB/RP.pm for LedgerSMB and SL/RP.pm for SQL-Ledger.

Other files exhibited similar probles but these have all been audited
and corrected in both programs, and it is not known how many other
issues were actually exploitable.

Here's the write-up for SQL-Ledger 2.8.33:

In rp.pl, sub report, line 415, you have the following line:

$form->{db} = ($form->{report} =~ /_collected/) ? "ar" : "ap";

What this does is place the strings either of 'ar' or 'ap' in the
hidden 'db' input of the form for resubmission.  The browser is
supposed to send back this data to the web server which then assumes
that this is either ar or ap.

This specific example is for this one only gets triggered when nextsub
= generate_tax_report....   The first line there is
RP->tax_report(\%myconfig, \%$form);

which proceeds to run queries assuming that $form->{db} is safe.

However if I send a url something where the action is
generate_tax_report and db contains arbitrary SQL code I should be
able to cause that to execute in the database in the query in RP.pm
beginning in line 1935.  Whatever $form->{db} is gets directly
interpolated in and there is no whitelisting to ensure it is always
'ar' or 'ap.'  The line where this occurs is 1941, and it reads JOIN
$form->{db} a ON (a.id = ac.trans_id)

So if the query string includes the url encoded equivalent of &db=ar a
join customer n on ar.id=customer.id; delete from audittrail; select *
from ar a join acc_trans ac on ar.id = ac.trans_id

Then the first part of the query would read:
SELECT a.id, a.invoice, $transdate AS transdate,
               a.invnumber, n.name, n.${vc}number, a.netamount,
               a.description,
               sum(ac.amount) * $ml AS tax,
               a.till, n.id AS vc_id
               FROM acc_trans ac
             JOIN ar a join customer n on ar.id=customer.id; delete
from audittrail; select * from ar a join acc_trans ac on ar.id =
ac.trans_id
              JOIN chart ch ON (ch.id = ac.chart_id)
             JOIN $vc n ON (n.id = a.${vc}_id)
               WHERE $where

We can reduce that to:
SELECT a.id, a.invoice, $transdate AS transdate,
               a.invnumber, n.name, n.${vc}number, a.netamount,
               a.description,
               sum(ac.amount) * $ml AS tax,
               a.till, n.id AS vc_id
               FROM acc_trans ac
             JOIN ar a join customer n on ar.id=customer.id;

delete from audittrail;

select * from ar a
 join acc_trans ac on ar.id = ac.trans_id
              JOIN chart ch ON (ch.id = ac.chart_id)
             JOIN $vc n ON (n.id = a.${vc}_id)
               WHERE $where

With a little effort, the audit trail could be deleted or other
database operations performed with the full permissions of the
application.  Neither LedgerSMB 1.2 nor SQL-Ledger 2.8 have generic
permissions safeguards in the database to prevent this sort of thing
(like most web applications, the application runs under a single
database user).  Consequently any SQL injection can be used to insert,
update, or delete data, for example in order to hide embezzlement or
theft.  Thus all such vulnerabilities should be considered critical.

Only the line numbers would be different for LedgerSMB 1.2.

Users of LedgerSMB prereleases for 1.3 are less vulnerable since audit
trails and other data have permissions enforced per user, thus
ensuring that sql injection issues do not pose the privilege
escalation issues that are present in prior versions.  Thus the impact
of an attack like this is greatly limited.  The impact on the
pre-releases should be seen as moderate.

Best Wishes,
Chris Travers