Bug 37322 - format bug: sprintf() twice on top of each other
format bug: sprintf() twice on top of each other
Status: RESOLVED FIXED
Product: Tomcat Connectors
Classification: Unclassified
Component: Common
unspecified
All All
: P4 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2005-11-01 17:50 UTC by J
Modified: 2008-10-05 03:09 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description J 2005-11-01 17:50:59 UTC
Hi,

mod_jk, more precisely
jakarta-tomcat-connectors-1.2.14.1-src/jk/native/apache-2.0/mod_jk.c
jakarta-tomcat-5.5.9-src/jakarta-tomcat-connectors/jk/native/apache-2.0/mod_jk.c
contain the following format security bug.

The good news is that it currently cannot be easily misused, because
the function involved is so far only used with the format string
"Memory Error" -- not a single "%" therein.
However it's just sitting there, waiting to be put to other uses.

jk_error_exit() calls apr_pvsprintf() to create a formatted string
(possibly using varargs) that is itself passed to ap_log_error().
Sadly, the latter function again uses sprintf-style formatting, so
that if a %-sign were obtained as a result of the first round of
formatting (e.g. as part of the output of some %s directive),
undefined behaviour could occur in the second pass.

Proposed solution:
"inline" the jk_error_code(), i.e. turn
  jk_error_exit(APLOG_MARK, APLOG_EMERG, s, p, "Memory error");
into:
 // use ap_log_[p/r/c]error(...) depending on availability of
 // either p (pool), s (server_rec *) or r (conn_rec *) at the call site.
 {
   ap_log_perror(APLOG_MARK, APLOG_EMERG, 0, p, "Memory error");
   exit(1);
 }

I found this bug while performing a code inspection on behalf of BSI,
the german Federal Office for Information Security.
 -- Bundesamt für Sicherheit in der Informationstechnik.
http://www.bsi.de

BSI endorses the use of Open Source Software. A report of our
activities covering installation, code and penetration tests of Tomcat
will be published by BSI in the internet at the end of our review
project.

Regards,
 Jörg Höhle.
T-Systems International, Solution Center Testing & Security
http://www.t-systems.com
Comment 1 Mladen Turk 2006-03-17 08:46:36 UTC
Like you said:

The good news is that it currently cannot be easily misused, because
the function involved is so far only used with the format string
"Memory Error" -- not a single "%" therein.
However it's just sitting there, waiting to be put to other uses.

So, we'll be paranoid if someone puts "%" instead "Memory error"
Comment 2 Rainer Jung 2008-01-01 16:02:53 UTC
An additional safety net has been introduced, any remaining special formatting
character gets replaced now. Will be part of 1.2.27.