Skip to content

Commit c637bf1

Browse files
LebedevRIpmjdebruijn
authored andcommitted
dt_iop_load_modules_so(): Prevent stack-buffer-overflow if IOP name is longer than 19 symbols
We use char op[20]; to store iop names, and if somehow we end up with iop with long-enough name, we end up overflowing stack: ==23602==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff00816974 at pc 0x7fdcbfbce3f2 bp 0x7fff00816810 sp 0x7fff00816808 WRITE of size 1 at 0x7fff00816974 thread T0 #0 0x7fdcbfbce3f1 in dt_iop_load_modules_so /home/lebedevri/darktable/src/develop/imageop.c:1214 #1 0x7fdcbfb00229 in dt_init /home/lebedevri/darktable/src/common/darktable.c:873 #2 0x400b9f in main /home/lebedevri/darktable/src/main.c:24 #3 0x7fdcb80b1b44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44) #4 0x400c22 (/usr/local/bin/darktable+0x400c22) Address 0x7fff00816974 is located in stack of thread T0 at offset 180 in frame #0 0x7fdcbfbcc92f in dt_iop_load_modules_so /home/lebedevri/darktable/src/develop/imageop.c:1196 This frame has 5 object(s): [32, 40) 'stmt' [96, 104) 'stmt' [160, 180) 'op' <== Memory access at offset 180 overflows this variable [224, 1248) 'path' [1280, 5376) 'plugindir' SUMMARY: AddressSanitizer: stack-buffer-overflow /home/lebedevri/darktable/src/develop/imageop.c:1214 dt_iop_load_modules_so ... ==23602==ABORTING Now, if we encounter such an iop (e.g. libexposure123456789012.so), we will simply fail to load it: [iop_load_module] failed to open operation `exposure12345678901': /usr/local/lib/darktable/plugins/libexposure12345678901.so: cannot open shared object file: No such file or directory While there, also add check to the CMake add_iop() macro to prevent adding such iops. (cherry picked from commit 8bfcc8c)
1 parent 5f0dd44 commit c637bf1

File tree

2 files changed

+6
-2
lines changed

2 files changed

+6
-2
lines changed

src/develop/imageop.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,8 +1211,7 @@ void dt_iop_load_modules_so()
12111211
// get lib*.so
12121212
if(!g_str_has_prefix(d_name, SHARED_MODULE_PREFIX)) continue;
12131213
if(!g_str_has_suffix(d_name, SHARED_MODULE_SUFFIX)) continue;
1214-
strncpy(op, d_name + name_offset, strlen(d_name) - name_end);
1215-
op[strlen(d_name) - name_end] = '\0';
1214+
g_strlcpy(op, d_name + name_offset, MIN(sizeof(op), strlen(d_name) - name_end + 1));
12161215
module = (dt_iop_module_so_t *)calloc(1, sizeof(dt_iop_module_so_t));
12171216
gchar *libname = g_module_build_path(plugindir, (const gchar *)op);
12181217
if(dt_iop_load_module_so(module, libname, op))

src/iop/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ cmake_minimum_required(VERSION 2.6)
33
include_directories("${CMAKE_CURRENT_BINARY_DIR}/../" "${CMAKE_CURRENT_SOURCE_DIR}")
44

55
macro (add_iop _lib _src)
6+
string(LENGTH ${_lib} _lib_namelength)
7+
if(${_lib_namelength} GREATER 19)
8+
message(FATAL_ERROR "IOP name \"${_lib}\" is too long (${_lib_namelength} symbols), it should not be greater than 19 symbols.")
9+
endif(${_lib_namelength} GREATER 19)
10+
611
set(_input ${CMAKE_CURRENT_SOURCE_DIR}/${_src})
712
set(_output ${CMAKE_CURRENT_BINARY_DIR}/introspection_${_src}) # keep ${_src} in the end to keep the file extension (c vs. cc)
813
add_custom_command(

0 commit comments

Comments
 (0)