BUG: Segfault due to incorrect checking for strerror_r in motion_log (motion.c)

#if (defined(BSD)) is wrong. According to the manual, it could be #if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE) to check if we have the XSI-compliant strerror_r or GNU-specific.

This bug causes Segmentation Fault because program expects GNU-specific str_error which returns char*, although XSI-compliant version is used on my system, which returns int. strcat() fails.

---

[1] Error reading first header - re-trying
[1] Error reading first header - re-trying

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fb92e297950 (LWP 27784)]
__strcat_chk (dest=0x7fb92e2952f0 "[1] connect() failed (106): ", src=0x0, 
    destlen=995) at strcat_chk.c:52
52   strcat_chk.c: No such file or directory.
   in strcat_chk.c
(gdb) bt
#0  __strcat_chk (dest=0x7fb92e2952f0 "[1] connect() failed (106): ", src=0x0, 
    destlen=995) at strcat_chk.c:52
#1  0x0000000000404437 in motion_log (level=3, errno_flag=1, 
    fmt=0x4248b7 "connect() failed (%d)") at /usr/include/bits/string3.h:145
#2  0x000000000040e9cd in netcam_connect (netcam=0x65bd60, err_flag=0)
    at netcam.c:933
#3  0x000000000040f10c in netcam_start (cnt=0x655030) at netcam.c:1922
#4  0x000000000040c390 in vid_start (cnt=0x7fb92e2952f0) at video_common.c:860
#5  0x0000000000406138 in motion_loop (arg=0x655030) at motion.c:572
#6  0x00007fb92f89b017 in start_thread (arg=<value optimized out>)
    at pthread_create.c:297
#7  0x00007fb92e577f7d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#8  0x0000000000000000 in ?? ()
(gdb) 

Another problem is that function syslog expects format and arguments, while it receives only format which is set from a variable.
That could be a string formatting vulnerability.

Please apply my patch below which solves both issues.

--- motion.c   2009-04-18 19:49:02.000000000 +0200
+++ motion.c   2009-04-18 20:08:52.000000000 +0200
@@ -2738,7 +2738,7 @@
 {
     int errno_save, n;
     char buf[1024];
-#if (!defined(BSD))
+#if (!((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE))
     char msg_buf[100];
 #endif
     va_list ap;
@@ -2772,7 +2772,7 @@
          * version of strerror_r, which doesn't actually put the message into
          * my buffer :-(.  I have put in a 'hack' to get around this.
          */
-#if (defined(BSD))
+#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE)
         strerror_r(errno_save, buf + n, sizeof(buf) - n);    /* 2 for the ': ' */
 #else
         strcat(buf, strerror_r(errno_save, msg_buf, sizeof(msg_buf)));
@@ -2780,7 +2780,7 @@
     }
     /* If 'level' is not negative, send the message to the syslog */
     if (level >= 0)
-        syslog(level, buf);
+        syslog(level, "%s", buf);
 
     /* For printing to stderr we need to add a newline */
     strcat(buf, "\n");

Environment

Motion version:	3.2.11
ffmpeg version:
Shared libraries:	ffmpeg, mysql, postgresql
Server OS:

-- MoronicRegistration - 18 Apr 2009

Follow up

Please can you attach as a diff ? your patch has some "weirds" html entities .

Please fill "Server OS", thanks !

-- AngelCarpintero - 09 Jul 2009

Fix record

Fixed in 3.2.11.1

-- AngelCarpintero - 10 Aug 2009