Segmentation fault: shcat built_in

[KevinMordijck]

when running shcat on a large file: shcat < documentationMitsuba.txt a segmentation fault occurs.

this is the end of the output from gdb:

13.2.5. Taking control of the logging system Many operations in Mitsuba will print one or more log messages during their execution. By default, they will be printed to the console, which may be undesirable. Similar to the C++ side, it is possible to define custom Formatter and Appender classes to interpret and direct the flow of these messages. This is also useful to keep track of the progress of rendering jobs. Roughly, a Formatter turns detailed information about a logging event into a human-readable string, and a Appender routes it to some destination (e.g. by appending it to a file or a log viewer in a graphical user interface). Here is an example of how to activate such extensions: import  mitsuba from  mitsuba.core  import  * class  MyFormatter(Formatter):         def  format(self,  logLevel,  sourceClass,  sourceThread,  message,  filename,  line):

Program received signal SIGSEGV, Segmentation fault. 0x00007ffff78198f3 in _IO_vfprintf_internal (s=, format=, ap=ap@entry=0x7fffffffda68) at vfprintf.c:1661 1661 vfprintf.c: No such file or directory. (gdb)

[KevinMordijck]

This is the output on the same file only the problem lines extracted. with jsh debug info on

Many operations in Mitsuba will print one or more log messages during their execution. By default, [DEBUG: stdin: now parsing line 2: 'they will be printed to the console, which may be undesirable. Similar to the C++ side, it is possible'] they will be printed to the console, which may be undesirable. Similar to the C++ side, it is possible [DEBUG: stdin: now parsing line 3: 'to define custom Formatter and Appender classes to interpret and direct the flow of these messages.'] to define custom Formatter and Appender classes to interpret and direct the flow of these messages. [DEBUG: stdin: now parsing line 4: 'This is also useful to keep track of the progress of rendering jobs.'] This is also useful to keep track of the progress of rendering jobs. [DEBUG: stdin: now parsing line 5: 'Roughly, a Formatter turns detailed information about a logging event into a human-readable'] Roughly, a Formatter turns detailed information about a logging event into a human-readable [DEBUG: stdin: now parsing line 6: 'string, and a Appender routes it to some destination (e.g. by appending it to a file or a log viewer in'] string, and a Appender routes it to some destination (e.g. by appending it to a file or a log viewer in [DEBUG: stdin: now parsing line 7: 'a graphical user interface). Here is an example of how to activate such extensions:'] a graphical user interface). Here is an example of how to activate such extensions: [DEBUG: stdin: now parsing line 8: 'import  mitsuba'] import  mitsuba [DEBUG: stdin: now parsing line 9: 'from  mitsuba.core  import  '] from  mitsuba.core  import  [DEBUG: stdin: now parsing line 10: 'class  MyFormatter(Formatter):'] class  MyFormatter(Formatter): [DEBUG: stdin: now parsing line 11: '        def  format(self,  logLevel,  sourceClass,  sourceThread,  message,  filename,  line):']         def  format(self,  logLevel,  sourceClass,  sourceThread,  message,  filename,  line): [DEBUG: stdin: now parsing line 12: '                return  '%s  (log  level:  %s,  thread:  %s,  class  %s,  file  %s,  line  %i)'  %  \'] Segmentation fault

[KevinMordijck]

this is the problem line: %s  (log  level:  %s,  thread:  %s,  class  %s,  file  %s,  line  %i)

[jovanbulck]

Oh :O I see a format string vulnerability! :-(

Nasty one, using the parsestream(FILE *strm, char* name, void (*fct)(char*)) function in jsh-common that takes a function as an argument and passes each line verbatim to it. Thus passing printf to this function, introduces (invisible) format string vulnerabilities...

Also try including things as %s%s%d%s%l%d in your jsh_login file to see some interesting things happening :/

Now writing a patch. Give me a minute...

[jovanbulck]

Done! Fixed the format string vulnerabilities in master and released a new bugfix release jsh 1.1.1 which is just a patched jsh 1.1.0 (no new features released yet).

FYI: here's the patch

From 84412df1e60ed363fb561b86bf8da0081b8a4984 Mon Sep 17 00:00:00 2001
From: Jo Van Bulck <jo.vanbulck@student.kuleuven.be>
Date: Thu, 6 Nov 2014 00:16:34 +0100
Subject: [PATCH] Fixed a format string vulnerability in parsestream() function

using the parsestream(FILE *strm, char* name, void (*fct)(char*)) function in jsh-common
that takes a function as an argument and passes each line verbatim to it.
Thus passing printf to this function, introduces (invisible) format string vulnerabilities...

Fix avoids using printf, by defining a wrapper function in jsh-common that used fputs (to avoid having puts
appending newlines)
---
 jsh-common.c | 16 ++++++++++++++--
 jsh-common.h |  6 ++++++
 jsh.c        |  4 ++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/jsh-common.c b/jsh-common.c
index 650ab20..0247d0c 100644
--- a/jsh-common.c
+++ b/jsh-common.c
@@ -89,10 +89,18 @@ char *gethome() {
     return (rv != NULL)? rv: curdir;
 }

+inline int puts_verbatim(const char *s) {
+    return fputs(s, stdout);
+}
+
 /*
  * parsefile: wrapper for parsestream(), opening and closing the file at the provided path.
- *  @arg errmsg: true  = print an error message if opening the file failed
+ * @arg errmsg: true  = print an error message if opening the file failed
  *               false = exit silently if opening the file failed
+ * @NOTE: if you pass a pointer to 'printf()' here, this may introduce format-string-
+ *  vulnerabilities as the lines are passed verbatim to the function. If you want to use
+ *  this function to print the content of a file line per line, pass a pointer to
+ *  'puts_verbatim()' (defined in jsh-common.h) instead.
  */
 void parsefile(char *path, void (*fct)(char*), bool errmsg) {
     FILE *file = fopen(path, "r");
@@ -106,7 +114,11 @@ void parsefile(char *path, void (*fct)(char*), bool errmsg) {

 /*
  * parsestream: reads the provided stream strm line per line, passing each line, 
- *  including '\n' to the provided function fct
+ *  including '\n' to the provided function fct.
+ * @NOTE: if you pass a pointer to 'printf()' here, this may introduce format-string-
+ *  vulnerabilities as the lines are passed verbatim to the function. If you want to use
+ *  this function to print the content of a file line per line, pass a pointer to
+ *  'puts_verbatim()' (defined in jsh-common.h) instead.
  */
 void parsestream(FILE *strm, char* name, void (*fct)(char*)) {
     printdebug("-------- now parsing stream '%s' --------", name);
diff --git a/jsh-common.h b/jsh-common.h
index 42dc45b..e7485f0 100644
--- a/jsh-common.h
+++ b/jsh-common.h
@@ -57,6 +57,12 @@
         fprintf(stream, "%s\n", NONE); \
     else \
         fprintf(stream, "\n"); \
+        
+/*
+ * A function that prints the provided '\0' terminated string verbatim on stdout, without 
+ *  appending a newline '\n' character.
+ */
+inline int puts_verbatim(const char *s);

 // ######### linux tty color codes ########
 #define RESET              0
diff --git a/jsh.c b/jsh.c
index d738f62..42d0076 100644
--- a/jsh.c
+++ b/jsh.c
@@ -251,7 +251,7 @@ void things_todo_at_start(void) {
         bool temp = DEBUG;
         DEBUG = false;        
         char * path = concat(3, gethome(), "/", LOGIN_FILE);
-        parsefile(path, (void (*)(char*)) &printf, false);
+        parsefile(path, (void (*)(char*)) puts_verbatim, false);
         free(path);
         DEBUG = temp;
         printdebug("debugging is on. Turn it off with 'debug off'.");
@@ -551,7 +551,7 @@ int parse_built_in(comd *comd, int index) {
             break;
             }
         case SHCAT:
-            parsestream(stdin, "stdin", (void (*)(char*)) printf);  // built_in cat; mainly for testing purposes (redirecting stdin)
+            parsestream(stdin, "stdin", (void (*)(char*)) puts_verbatim);  // built_in cat; mainly for testing purposes (redirecting stdin)
             return EXIT_SUCCESS;
             break;
         case UNALIAS:
-- 
2.1.2