Bug 727644 - (CVE-2011-3200) CVE-2011-3200 rsyslog: parseLegacySyslogMsg off-by-two buffer overflow
CVE-2011-3200 rsyslog: parseLegacySyslogMsg off-by-two buffer overflow
Status: CLOSED ERRATA
Product: Security Response
Classification: Other
Component: vulnerability (Show other bugs)
unspecified
Unspecified Unspecified
medium Severity medium
: ---
: ---
Assigned To: Red Hat Product Security
impact=moderate,public=20110901,repor...
: Security
Depends On: 733647 733648 735205
Blocks: 727687
  Show dependency treegraph
 
Reported: 2011-08-02 13:51 EDT by Martin Osvald
Modified: 2016-11-08 11:22 EST (History)
7 users (show)

See Also:
Fixed In Version: rsyslog 4.6.8, rsyslog 5.8.5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-10-06 08:27:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
upstream fix (503 bytes, patch)
2011-08-11 06:45 EDT, Rainer Gerhards
no flags Details | Diff
v5 patch (475 bytes, patch)
2011-08-11 08:28 EDT, Rainer Gerhards
no flags Details | Diff

  None (edit)
Description Martin Osvald 2011-08-02 13:51:19 EDT
Description of problem:

If we send to syslog a specially crafted log message, stack guard variable protecting bufParseTAG array in parseLegacySyslogMsg() gets rewritten and rsyslog gets terminated.


Version-Release number of selected component (if applicable):

rsyslog-4.6.2-3.el6_1.1.i686


How reproducible:

everytime on ia-32
on x86-64 stack guard variable won't get rewritten as it is located 8 bytes above bufParseTAG array


Actual results:

glibc prints the following message and rsyslog gets terminated:

*** stack smashing detected ***: rsyslogd terminated


Expected results:

No abortion.


Additional info:

For more info, please, see the following private comment.
Comment 1 Martin Osvald 2011-08-02 13:52:40 EDT
###############################################

The following is investigation of corefile from customer before finding the reproducer:

let's find the address of guard variable:

(gdb) disassemble parseLegacySyslogMsg
...
   0x00d74fc1 <+33>:    mov    %gs:0x14,%eax
   0x00d74fc7 <+39>:    mov    %eax,-0x1c(%ebp)
...
   0x00d75048 <+168>:   mov    -0x1c(%ebp),%edx
   0x00d7504b <+171>:   xor    %gs:0x14,%edx
   0x00d75052 <+178>:   jne    0xd75411 <parseLegacySyslogMsg+1137>
...
   0x00d75411 <+1137>:  call   0xda9140 <__stack_chk_fail_local>


from above we can see that it is stored 28 (0x1c) bytes below the base pointer and it is stored exactly above bufParseTAG array:

(gdb) f 12
#12 0x00d75416 in parseLegacySyslogMsg (pMsg=0xb4301338, flags=48) at syslogd.c:1322
1322    }
(gdb) p &bufParseTAG
$1 = (uchar (*)[512]) 0xb7871f4c
(gdb) x/x 0xb7871f4c+512
0xb787214c:     0x45092900
(gdb) x/x $ebp-0x1c
0xb787214c:     0x45092900
(gdb)


If we look at the code which fills in the bufParseTAG buffer in parseLegacySyslogMsg() we can find the following code:

1189 int parseLegacySyslogMsg(msg_t *pMsg, int flags)
1190 {
1191         uchar *p2parse;
1192         int lenMsg;
...
1194         int i;  /* general index for parsing */
1195         uchar bufParseTAG[CONF_TAG_MAXSIZE];
...
1201         lenMsg = pMsg->iLenRawMsg - pMsg->offAfterPRI; /* note: offAfterPRI is already the number of PRI chars (do not add one!) */
...
1240         if(bParseHOSTNAMEandTAG && !(flags & INTERNAL_MSG)) {
...
1293                 i = 0;
1294                 while(lenMsg > 0 && *p2parse != ':' && *p2parse != ' ' && i < CONF_TAG_MAXSIZE) {
1295                         bufParseTAG[i++] = *p2parse++;
1296                         --lenMsg;
1297                 }
1298                 if(lenMsg > 0 && *p2parse == ':') {
1299                         ++p2parse;
1300                         --lenMsg;
1301                         bufParseTAG[i++] = ':';
1302                 }
1303 
1304                 /* no TAG can only be detected if the message immediatly ends, in which case an empty TAG
1305                  * is considered OK. So we do not need to check for empty TAG. -- rgerhards, 2009-06-23
1306                  */
1307                 bufParseTAG[i] = '\0';  /* terminate string */
1308                 MsgSetTAG(pMsg, bufParseTAG, i);


and in conjunction with the fact that lenMsg variable had a value of 674 at the beginning of parseLegacySyslogMsg():

(gdb) p pMsg->iLenRawMsg 
$6 = 679
(gdb) p pMsg->offAfterPRI
$7 = 5
(gdb)

1201         lenMsg = pMsg->iLenRawMsg - pMsg->offAfterPRI; /* note: offAfterPRI is already the number of PRI chars (do not add one!) */


and value 162 at its end

(gdb) p lenMsg
$5 = 162
(gdb)


and according to malformed message which doesn't contain hostname in the required form:

(gdb) p pMsg-> pszRawMsg
$4 = (
    uchar *) 0xb43014e0 "<174>...<real log message content removed>..."...
(gdb)


it must have happened that the following condition was skipped (also taken from parseLegacySyslogMsg()):

1255                 if(lenMsg > 0 && flags & PARSE_HOSTNAME) {
1256                         i = 0;
1257                         while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.'
1258                                 || p2parse[i] == '_' || p2parse[i] == '-') && i < (CONF_HOSTNAME_MAXSIZE - 1)) {


which led to iterating the loop for maximum allowed times (CONF_TAG_MAXSIZE = 512):

1294                 while(lenMsg > 0 && *p2parse != ':' && *p2parse != ' ' && i < CONF_TAG_MAXSIZE) {
1295                         bufParseTAG[i++] = *p2parse++;
1296                         --lenMsg;
1297                 }


which led to 'i' variable having a value of 512 at the end of above while loop which finally led to overwriting 1 byte memory after bufParseTAG array with zero:

1307                 bufParseTAG[i] = '\0';  /* terminate string */


We can see that the guard variable has one byte zeroed (and that is why the stack smashing protection terminated the process):

(gdb) x/xw 0xb787214c
0xb787214c:     0x45092900
(gdb) x/c 0xb787214c
0xb787214c:     0 '\000'
(gdb)


###############################################

The following are results if we use the mentioned reproducer:

# cat ../../rsyslog.batch 
set environment RSYSLOG_DEBUG debug nostdout
b parseLegacySyslogMsg
#
# gdb ./rsyslog-4.6.2/tools/rsyslogd -x ../../rsyslog.batch
...
Breakpoint 1 at 0x90bd: file syslogd.c, line 1190.
(gdb) r -c 4
Starting program: /root/00506609/mosvald/rsyslog/RHEL-6/rsyslog-4.6.2/tools/rsyslogd -c 4
[Thread debugging using libthread_db enabled]
[New Thread 0xb7dfeb70 (LWP 20328)]

Breakpoint 1, parseLegacySyslogMsg (pMsg=0x17d168, flags=1) at syslogd.c:1190
1190    {
Missing separate debuginfos, use: debuginfo-install zlib-1.2.3-25.el6.i686
(gdb) c
Continuing.
[New Thread 0xb71ffb70 (LWP 20329)]
[New Thread 0xb67feb70 (LWP 20330)]
[Switching to Thread 0xb71ffb70 (LWP 20329)]

Breakpoint 1, parseLegacySyslogMsg (pMsg=0xb5c00cf8, flags=4) at syslogd.c:1190
1190    {
(gdb) n
1201            lenMsg = pMsg->iLenRawMsg - pMsg->offAfterPRI; /* note: offAfterPRI is already the number of PRI chars (do not add one!) */
(gdb) 
1202            p2parse = pMsg->pszRawMsg + pMsg->offAfterPRI; /* point to start of text, after PRI */
(gdb) 
1210            if(datetime.ParseTIMESTAMP3339(&(pMsg->tTIMESTAMP), &p2parse, &lenMsg) == RS_RET_OK) {
(gdb) p lenMsg
$1 = 512
(gdb) p p2parse
$2 = (uchar *) 0xb5c00ea5 '#' <repeats 200 times>...
(gdb) n
1212            } else if(datetime.ParseTIMESTAMP3164(&(pMsg->tTIMESTAMP), &p2parse, &lenMsg) == RS_RET_OK) {
(gdb) 
1214            } else if(*p2parse == ' ' && lenMsg > 1) { /* try to see if it is slighly malformed - HP procurve seems to do that sometimes */
(gdb) 
1228            if(flags & IGNDATE) {
(gdb) 
1230                    memcpy(&pMsg->tTIMESTAMP, &pMsg->tRcvdAt, sizeof(struct syslogTime));
(gdb) 
1240            if(bParseHOSTNAMEandTAG && !(flags & INTERNAL_MSG)) {
(gdb) 
1254                    bTAGCharDetected = 0;
(gdb) 
1255                    if(lenMsg > 0 && flags & PARSE_HOSTNAME) {
(gdb) 
1293                    i = 0;
(gdb) 
1294                    while(lenMsg > 0 && *p2parse != ':' && *p2parse != ' ' && i < CONF_TAG_MAXSIZE) {
(gdb) 
1295                            bufParseTAG[i++] = *p2parse++;
(gdb) 
1296                            --lenMsg;
(gdb) list
1291                     * outputs so that only 32 characters max are used by default.
1292                     */
1293                    i = 0;
1294                    while(lenMsg > 0 && *p2parse != ':' && *p2parse != ' ' && i < CONF_TAG_MAXSIZE) {
1295                            bufParseTAG[i++] = *p2parse++;
1296                            --lenMsg;
1297                    }
1298                    if(lenMsg > 0 && *p2parse == ':') {
1299                            ++p2parse; 
1300                            --lenMsg;
(gdb) b 1298
Breakpoint 2 at 0x1194cf: file syslogd.c, line 1298.
(gdb) c
Continuing.

Breakpoint 2, parseLegacySyslogMsg (pMsg=0xb5c00cf8, flags=4) at syslogd.c:1298
1298                    if(lenMsg > 0 && *p2parse == ':') {
(gdb) n
1307                    bufParseTAG[i] = '\0';  /* terminate string */
(gdb) p i
$3 = 512
(gdb) p lenMsg
$4 = 0
(gdb) disassemble parseLegacySyslogMsg 
...
   0x001190bd <+30>:    mov    %gs:0x14,%eax
   0x001190c3 <+36>:    mov    %eax,-0xc(%ebp)
...
(gdb) x/x $ebp-0xc
0xb71fdd1c:     0x78462c4e
(gdb) n
1308                    MsgSetTAG(pMsg, bufParseTAG, i);
(gdb) x/x $ebp-0xc
0xb71fdd1c:     0x78462c00
(gdb) c
Continuing.
*** stack smashing detected ***: /root/00506609/mosvald/rsyslog/RHEL-6/rsyslog-4.6.2/tools/rsyslogd terminated
...
(gdb)
Comment 3 Martin Osvald 2011-08-02 15:19:33 EDT
forgot to mention backtrace, here it is:

(gdb) bt
#0  __kernel_vsyscall () at arch/x86/vdso/vdso32/sysenter.S:49
#1  0x00359d11 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x0035b5ea in abort () at abort.c:92
#3  0x00d8d28e in sigsegvHdlr (signum=6) at debug.c:831
#4  <signal handler called>
#5  __kernel_vsyscall () at arch/x86/vdso/vdso32/sysenter.S:49
#6  0x00359d11 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#7  0x0035b5ea in abort () at abort.c:92
#8  0x00397b9d in __libc_message (do_abort=2, fmt=0x47c3d2 "*** %s ***: %s terminated\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:186
#9  0x00426fed in __fortify_fail (msg=0x47c3ba "stack smashing detected") at fortify_fail.c:32
#10 0x00426f9a in __stack_chk_fail () at stack_chk_fail.c:29
#11 0x00da9154 in __stack_chk_fail_local ()
#12 0x00d75416 in parseLegacySyslogMsg (pMsg=0xb4301338, flags=48) at syslogd.c:1322
#13 0x00d82bf5 in parseMsg (pMsg=0xb4301338) at parser.c:323
#14 0x00d74967 in msgConsumer (notNeeded=0x0, pUsr=0xb4301338) at syslogd.c:941
#15 0x00d9d67b in qqueueConsumerReg (pThis=0x15f3930, pWti=0x15ec820, iCancelStateSave=0) at queue.c:1609
#16 0x00d980f2 in wtiWorker (pThis=0x15ec820) at wti.c:406
#17 0x00d97661 in wtpWorker (arg=0x15ec820) at wtp.c:463
#18 0x00fb89e9 in start_thread (arg=0xb7872b70) at pthread_create.c:301
#19 0x0040ccde in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:133
(gdb
Comment 4 Vincent Danen 2011-08-02 16:35:15 EDT
This seems to only be a problem on RHEL6.  Using the above reproducer, I cannot reproduce on F15 or RHEL5 (rsyslog is available but not the default on RHEL5).
Comment 5 Vincent Danen 2011-08-02 16:44:20 EDT
After thinking about this further, we just need confirmation as to whether or not this can be used to cause arbitrary code execution since rsyslogd runs as root.  I suspect SELinux would help us if it did.  At best this is a DoS since someone could use this to kill the system logger before doing other things (to help cover their tracks; especially useful if rsyslog is configured to send logs to a central log server).

I'm going to change this to an SRT bug since either way there is a vulnerability here.

Do we know if this can only be triggered locally, or if it can be triggered remotely as well?
Comment 6 Tomas Hoger 2011-08-03 04:47:48 EDT
(In reply to comment #1)
> which led to 'i' variable having a value of 512 at the end of above while loop
> which finally led to overwriting 1 byte memory after bufParseTAG array with
> zero:
> 
> 1307                 bufParseTAG[i] = '\0';  /* terminate string */

I think we can sum comment #1 as: the crash was caused by an off-by-one buffer overflow, which touched SSP guard cookie and resulted in SSP abort.  Well, it's actually off-by-two in some cases (see lines 1298 - 1301), but the overflow is limited to "\0" or ":\0".  Test with buf='<174>'+'#'*512+':' in your reproducer.  We should make the relevant info from comment #1 public when this bug is opened.

(In reply to comment #5)
> After thinking about this further, we just need confirmation as to whether or
> not this can be used to cause arbitrary code execution since rsyslogd runs as
> root.

The overflow is too short here.  When SSP is used, gcc changes order of the local variables.  Hence all other local variables in parseLegacySyslogMsg() will be placed below bufParseTAG[] (and bufParseHOSTNAME[]) and can not be overwritten.  Overflow can touch SSP cookie, or not even reach it (as noted in comment #0).  SSP mitigates to crash-only.
Comment 7 Tomas Hoger 2011-08-03 04:50:03 EDT
(In reply to comment #2)
> I do not provide temporary fix even it would be consisted of restricting value
> of 'i' variable after the line #1297

i < CONF_TAG_MAXSIZE - 2, similar to i < (CONF_HOSTNAME_MAXSIZE - 1) possibly?
Comment 9 Tomas Heinrich 2011-08-03 10:04:35 EDT
(In reply to comment #5)
> Do we know if this can only be triggered locally, or if it can be triggered
> remotely as well?

If the daemon is configured to receive messages through udp or tcp, it can be triggered by using netcat from remote.
Comment 10 Vincent Danen 2011-08-03 12:44:05 EDT
(In reply to comment #9)
> (In reply to comment #5)
> > Do we know if this can only be triggered locally, or if it can be triggered
> > remotely as well?
> 
> If the daemon is configured to receive messages through udp or tcp, it can be
> triggered by using netcat from remote.

Ok, then we should probably ASYNC this once we get the fully list of what it affects (to be sure that some variation of the reproducer doesn't work on RHEL5, etc.) and a patch.  We should also coordinate a release with other vendors who may be shipping this as killing syslog is a great way to hide your tracks if you're planning to attack something.
Comment 11 Martin Osvald 2011-08-03 18:02:33 EDT
(In reply to comment #10)
> Ok, then we should probably ASYNC this once we get the fully list of what it
> affects (to be sure that some variation of the reproducer doesn't work on
> RHEL5, etc.)

RHEL-4 doesn't ship rsyslog

just checked the RHEL-5 code and it cannot happen there. It doesn't iterate over buffer stored on the stack, but allocated by malloc. It doesn't use extra variable as index to access elements of the buffer nor variable holding a number of remaining bytes, but accesses it directly by incrementing a pointer until it reaches zero and as it allocates extra byte for terminating '\0' there isn't any alteration of memory past the buffer, the only problem I can see is that the while loop isn't restricted to stop after strlen(p2parse) iterations or the last byte of pBuf isn't zeroed.

rsyslog-3.22.1/tools/syslogd.c:
1404 static int parseLegacySyslogMsg(msg_t *pMsg, int flags)
1405 {
1406         char *p2parse;
...
1416         p2parse = (char*) pMsg->pszUxTradMsg;
...
1466                 if(pMsg->bParseHOSTNAME) {
1467                         /* TODO: quick and dirty memory allocation */
1468                         /* the memory allocated is far too much in most cases. But on the plus side,
1469                          * it is quite fast... - rgerhards, 2007-09-20
1470                          */
1471                         if((pBuf = malloc(sizeof(char)* (strlen(p2parse) +1))) == NULL)
1472                                 return 1;
1473                         pWork = pBuf;
1474                         /* this is the actual parsing loop */
1475                         while(*p2parse && *p2parse != ' ' && *p2parse != ':') {
1476                                 if(*p2parse == '[' || *p2parse == ']' || *p2parse == '/')
1477                                         bTAGCharDetected = 1;
1478                                 *pWork++ = *p2parse++;
1479                         }
1480                         /* we need to handle ':' seperately, because it terminates the
1481                          * TAG - so we also need to terminate the parser here!
1482                          * rgerhards, 2007-09-10 *p2parse points to a valid address here in 
1483                          * any case. We can reach this point only if we are at end of string,
1484                          * or we have a ':' or ' '. What the if below does is check if we are
1485                          * not at end of string and, if so, advance the parse pointer. If we 
1486                          * are already at end of string, *p2parse is equal to '\0', neither if
1487                          * will be true and the parse pointer remain as is. This is perfectly
1488                          * well.
1489                          */
1490                         if(*p2parse == ':') {
1491                                 bTAGCharDetected = 1;
1492                                 /* We will move hostname to tag, so preserve ':' (otherwise we 
1493                                  * will needlessly change the message format) */
1494                                 *pWork++ = *p2parse++;
1495                         } else if(*p2parse == ' ')
1496                                 ++p2parse;
1497                         *pWork = '\0';
Comment 12 Vincent Danen 2011-08-04 11:58:43 EDT
(In reply to comment #11)
> (In reply to comment #10)
> > Ok, then we should probably ASYNC this once we get the fully list of what it
> > affects (to be sure that some variation of the reproducer doesn't work on
> > RHEL5, etc.)
> 
> RHEL-4 doesn't ship rsyslog
> 
> just checked the RHEL-5 code and it cannot happen there. It doesn't iterate
> over buffer stored on the stack, but allocated by malloc. It doesn't use extra
> variable as index to access elements of the buffer nor variable holding a

Perfect, thank you for checking!  So we only need to worry about this for RHEL6 then.  Have you looked at Fedora?  Maybe then this is only something that affects 4.x; I couldn't get it to reproduce on F15 (5.8.2), but have not tried Fedora 14.

Have you looked into a patch for this, or has this been reported to upstream at all?  If not, we should probably get in touch with them; it looks like 4.x is still supported.
Comment 13 Martin Osvald 2011-08-04 22:03:58 EDT
(In reply to comment #12)
> Have you looked at Fedora?  Maybe then this is only something that
> affects 4.x; I couldn't get it to reproduce on F15 (5.8.2), but have not tried
> Fedora 14.

Yes, on F-14 it is reproducible as it contains the same code as in RHEL6 (4.6.3), I can see that F-15 (5.8.2) also contains the wrong code, but it is little bit refactored so need to debug it first to see what is happening there, as soon as I get to it I will let you know if nobody will be faster than me.

> Have you looked into a patch for this

No, but I think Tomas's comment #9 could be reasonable solution.

> or has this been reported to upstream at all?

No, it hasn't been reported to anyone.
Comment 16 Vincent Danen 2011-08-08 10:29:05 EDT
Adding upstream (Rainer) to the bug.  Rainer, this is currently non-public, so please do not make any public commits to any repos, etc. without first coordinating a release date to unembargo with us (so that we have an opportunity to inform other vendors who may be shipping rsyslog before it is made public, so that they have the opportunity to patch it beforehand as well).

We have a preliminary patch idea in comment #7; do you think that fix would work?  Also, we've so far only been able to reproduce this on 4.x, but Martin has looked briefly at 5.x (comment #13) and we're not sure if it is affected, perhaps in a different way (or if something else is preventing it from being a problem).  It does not look like 3.x is affected, but confirmation of that from you would be fantastic.  Thank you!
Comment 17 Rainer Gerhards 2011-08-08 11:03:51 EDT
Thanks for working with us in getting this fixed. I have just begun to dig into the details and will probably need tomorrow to get to a decent response. I'll also check the other important parser (RFC5424), which may have a similar issue. Will post when I have more info.
Comment 18 Rainer Gerhards 2011-08-08 11:39:56 EDT
I have preliminary verified the problem in v4. The patch from comment #7 is a temporary solution. The problematic code is definitely contained in v5 and v6. Even if it does not trigger an abort, the off-by-1/2 can happen there, too. 

As far as I can quickly see, v3 is not affected as no limit for the tag was set there.
Comment 19 Vincent Danen 2011-08-10 16:23:50 EDT
Adding the Debian maintainer Michael Biebl as per Rainer's request.
Comment 20 Rainer Gerhards 2011-08-11 06:45:12 EDT
Created attachment 517770 [details]
upstream fix

The patch is identical to what has already been proposed on this bug tracker. Note that tt will effectively reduce the max supported tag size by two. However, the max size was choosen more or less random as an upper bound which by far is never expected to be seen in practice. So this doesn't really matter. If there are concerns, the max size can be increased in ./runtime/rsyslog.h.
Comment 21 Rainer Gerhards 2011-08-11 07:23:19 EDT
I have analyzed the problem in detail. First of all, this for sure is a bug that needs to be fixed. However, it seems to be very hard to reproduce. Actually, I did not manage on any other platform than described here.

It is hard to reproduce because it depends on many factors that need to go together. Let's have a look at the code (from ./tools/syslogd.c, current git v4-stable, ln 1189+):

1193         int i;  /* general index for parsing */
1194         uchar bufParseTAG[CONF_TAG_MAXSIZE];
1195         uchar bufParseHOSTNAME[CONF_HOSTNAME_MAXSIZE];

The buffer potentially overrun is bufParseTAG. Let's assume the compiler does not reorder variables. Then the following happens:
(a) on most platforms, bufParseHOSTNAME is overrun. This is no problem, because it is no longer used when this happens. Also bufParseTAG is strcpy()'ed right away, so this causes no issue at all.

(b) on some platforms (I had none), variable i is partially overwritten. This index is used shortly thereafter. Truncation of TAG can happen, but no abort:
(ba) on big endian machines, not follow-up error occurs, as the overwritten location continued 0 and the same value is written
(bb) on little endian machines, the lowest byte is overwritten with zero, leading to a truncation of the tag value.

I have not analysed all possible permutations if the compiler reorders. But obviously, some of the them can cause problems. The most important cases are overwriting the parse pointer or, as seen here, the return address (if bufParseTAG is located right after it).

This rearrangement happens with stack guard. Without knowing the implementation details, it looks like stack guard seems to reorder EITHER bufParseTAG or bufParseHOSTNAME directly after the guard variable. If it is bufParseTAG, the overrun will be detected and an abort happens. However, if it is buf ParseHOSTNAME, the problem will neither be detected nor cause any damage (as described above). Thus the problem is not always reproducible even if stack guard is consistenly active.

I don't see any theoretical evidence why 32 vs 64 bitness should make a difference, but the postings here suggest it does.

The bug was introduced in v4-stable with 4.6.0 (released 2010-02-24). It currently is present in all stable versions above that release (including v5). I have NOT checked development versions, as they are not suitable for production use in any case. But the bug has been introduced some time during the 4.5.x performance optimization effort.

In conclusion, I think that the actual environment and compile-time settings seem to have very important impact on the damage caused by this bug. It seems not to cause damage on the majority of platforms in use. That probably explains why it was kept undetected for so long.

As with all bugs of this kind, the fix is trivial and now attached to the tracker.

As a general advise, it is suggested to NOT accept syslog from the public Internet. If this has been forbidden, the damage potential is further restricted. If syslog must be accepted from the Internet, it is highly suggested to use RFC5424/5425 TLS-protected syslog with mutual authentication, which would prevent an attacker from injecting malicious messages.

Thanks again everyone for diagnosing this issue and working with us to fix it. At this point, I'll hand over to Andre Lorbach for coordination of the actual relases as I am heading to vacation tomorrow. If there are any further concerns, I'll try to answer them ASAP.
Comment 22 Rainer Gerhards 2011-08-11 07:29:16 EDT
I forgot to add: the 5424 parser is NOT affected, as it uses a totally different method relying on dynamically-sized buffers (and a sufficient size of them).
Comment 23 Tomas Hoger 2011-08-11 08:20:01 EDT
(In reply to comment #21)
> (a) on most platforms, bufParseHOSTNAME is overrun. This is no problem,
> because it is no longer used when this happens. Also bufParseTAG is
> strcpy()'ed right away, so this causes no issue at all.

This does not sound correct.  At least on x86 Linux, bufParseHOSTNAME would be below bufParseTAG, which would be below i.  Hence i should be overwrite in the most common Linux builds without stack protector.

> This rearrangement happens with stack guard. Without knowing the
> implementation details, it looks like stack guard seems to reorder EITHER
> bufParseTAG or bufParseHOSTNAME directly after the guard variable.

Stack protector should move all non-buffer variables below buffers.  This is to reduce the likelihood that some important variable is overwritten and used before stack canary is checked.  Hence all p2parse, lenMsg, bTAGCharDetected and i should be moved below the two buffers (unless optimized out), bufParseTAG and bufParseHOSTNAME should not get reordered.

> Thus the problem is not always reproducible even if stack guard is
> consistenly active.

Actually, it is reliably reproducible on at least current i686 RHEL-6 builds, leading to rsyslogd crash/abort.

> I don't see any theoretical evidence why 32 vs 64 bitness should make a
> difference, but the postings here suggest it does.

I'd suspect some memory alignment may come into play, but I've not tried to confirm that.

Thank you for providing the patch!
Comment 24 Rainer Gerhards 2011-08-11 08:28:37 EDT
Created attachment 517785 [details]
v5 patch

Bascially same patch, but file location changed. So for your conveience a patch for v5+.
Comment 25 Rainer Gerhards 2011-08-12 04:05:45 EDT
(In reply to comment #23)
> (In reply to comment #21)
> > (a) on most platforms, bufParseHOSTNAME is overrun. This is no problem,
> > because it is no longer used when this happens. Also bufParseTAG is
> > strcpy()'ed right away, so this causes no issue at all.
> 
> This does not sound correct.  At least on x86 Linux, bufParseHOSTNAME would be
> below bufParseTAG, which would be below i.  Hence i should be overwrite in the
> most common Linux builds without stack protector.

Looks like I mixed up the two cases - sorry for that.

...

> > Thus the problem is not always reproducible even if stack guard is
> > consistenly active.
> 
> Actually, it is reliably reproducible on at least current i686 RHEL-6 builds,
> leading to rsyslogd crash/abort.

Of course, but I was talking about different platforms. As I said, it is (consistently) reproducible on RHEL-6 builds and F14 (I think it was). But to me it is not consitently reproducible on all platforms. Just for clarification.
Comment 26 Vincent Danen 2011-08-20 11:26:08 EDT
When do we plan on making this public?  If we have a reproducer and a patch, should we not consider when we should be alerting other vendors and making this issue public?

What works for upstream here?  If we let vendor-sec know, they have a policy of embargoes no longer than 14 days.  Is there more to do yet, or can we plan for a release date (this also depends on upstream release schedules, etc.).

Also note that I won't be around next week at all (not to say we can't release something next week, just pointing out that my responses will be delayed).  Thanks.
Comment 35 Rainer Gerhards 2011-08-31 08:06:30 EDT
I am back from vacation and know that 9/1 was set as release date. We have prepared everything over here. Question now: at what time you usually release? We try to release security-related things around noon CET.
Comment 36 Tomas Hoger 2011-08-31 08:59:06 EDT
Upstream release around noon CE(S)T sounds fine for us, we're likely to release our updated packages within few hours after that.
Comment 37 Rainer Gerhards 2011-08-31 09:06:35 EDT
OK, so I'll go for 11a UTC (1p CEST). It's a bit closer to your release. Thanks for all your help!
Comment 38 Tomas Hoger 2011-09-01 12:22:18 EDT
Upstream advisory:
http://www.rsyslog.com/potential-dos-with-malformed-tag/

Fixed upstream in stable releases 4.6.8 or 5.8.5.

Upstream commit:
http://git.adiscon.com/?p=rsyslog.git;a=commitdiff;h=1ca6cc236d1dabf1633238b873fb1c057e52f95e
Comment 39 Vincent Danen 2011-09-01 15:43:48 EDT
Created rsyslog tracking bugs for this issue

Affects: fedora-all [bug 735205]
Comment 40 errata-xmlrpc 2011-09-01 16:01:54 EDT
This issue has been addressed in following products:

  Red Hat Enterprise Linux 6

Via RHSA-2011:1247 https://rhn.redhat.com/errata/RHSA-2011-1247.html
Comment 42 Vincent Danen 2011-10-07 10:56:06 EDT
External References:

http://www.rsyslog.com/potential-dos-with-malformed-tag/

Note You need to log in before you can comment on or make changes to this bug.