libexpat 2.1.0 flagged as a security risk by blackduck
nknmsc opened this issue · 12 comments
We use the fmi-library in ADAMS. One of the components used by fmi-library, libexpat.2.1.0 is flagged by BlackDuck as a security Risk.
I would like to know how to submit a request to update the libexpat.2.1.0 to a newer version to eliminate this security risk.
I sent an email to modelon-community@modelon.com but have not heard anything back. Hence I am posting this here.
Please help. Thank you.
We use the fmi-library in ADAMS. One of the components used by fmi-library, libexpat.2.1.0 is flagged by BlackDuck as a security Risk. I would like to know how to submit a request to update the libexpat.2.1.0 to a newer version to eliminate this security risk.
I sent an email to modelon-community@modelon.com but have not heard anything back. Hence I am posting this here.
Please help. Thank you.
Hi @nknmsc, I am looking into this and will get back to you.
Thank you for raising this issue.
On a slightly different note: It would be great, if it would be possible to provide the dependencies to the build instead of fmi-library building them.
One would simply have to use find_package from CMake to locate an existing package. If the dependency is missing, one could fall back to building it.
That way I could update to a newer version of expat without changes to the source code (assuming the new version has a compatible interface).
@modelonrobinandersson - I would like to know if there is an update on this. Thank you.
Hi, I'm afraid that there is no update on this at the moment.
On another note, if the FMU is untrusted, you will need to take steps (in the importing tool) to make sure that you can trust it. Both the binary as well as the XML and any other content in the FMU.
For anyone wondering, we used the patch below to build a version of fmi-library where zlib and expat are consumed through find_package. Line numbers might vary since we apply a number of patches before that. But you should get the idea. We applied the patch from #7 earlier, but this shouldn't be needed with this approach.
diff --git a/CMakeLists.txt b/CMakeLists.txt
index cabddd8..1b273a0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -230,7 +230,7 @@ configure_file (
"${FMILibrary_BINARY_DIR}/fmilib_config.h"
)
-set(FMILIB_SHARED_SUBLIBS ${FMIXML_LIBRARIES} ${FMIZIP_LIBRARIES} ${FMICAPI_LIBRARIES} expat minizip zlib c99snprintf)
+set(FMILIB_SHARED_SUBLIBS ${FMIXML_LIBRARIES} ${FMIZIP_LIBRARIES} ${FMICAPI_LIBRARIES} minizip c99snprintf)
set(FMILIB_SUBLIBS ${FMIIMPORT_LIBRARIES} ${JMUTIL_LIBRARIES} ${FMILIB_SHARED_SUBLIBS})
set(FMILIB_SHARED_SRC ${FMIIMPORTSOURCE} ${JMUTILSOURCE} ${FMIIMPORTHEADERS})
diff --git a/Config.cmake/Minizip/CMakeLists.txt b/Config.cmake/Minizip/CMakeLists.txt
index 9452915..c562b51 100644
--- a/Config.cmake/Minizip/CMakeLists.txt
+++ b/Config.cmake/Minizip/CMakeLists.txt
@@ -8,11 +8,6 @@ set(SKIP_INSTALL_FILES ON)
if(NOT FMILIB_INSTALL_SUBLIBS)
set(SKIP_INSTALL_LIBRARIES ON)
endif()
-add_subdirectory("${FMILIB_THIRDPARTYLIBS}/Zlib/zlib-1.2.6" "${FMILibrary_BINARY_DIR}/zlib")
-
-if(CMAKE_CL_64)
- set_target_properties(zlib PROPERTIES STATIC_LIBRARY_FLAGS "/machine:x64")
-endif()
if(CMAKE_HOST_APPLE)
set(PLATFORM __APPLE__)
@@ -30,7 +25,6 @@ if(CMAKE_COMPILER_IS_GNUCC)
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99")
endif()
-include_directories("${FMILIB_THIRDPARTYLIBS}/Zlib/zlib-1.2.6" "${FMILibrary_BINARY_DIR}/zlib")
set(SOURCE
${FMILIB_THIRDPARTYLIBS}/Minizip/minizip/ioapi.c
${FMILIB_THIRDPARTYLIBS}/Minizip/minizip/miniunz.c
@@ -58,7 +52,7 @@ endif(WIN32)
add_library(minizip ${SOURCE} ${HEADERS})
-target_link_libraries(minizip zlib)
+target_link_libraries(minizip PUBLIC CONAN_PKG::zlib)
if(FMILIB_INSTALL_SUBLIBS)
install(TARGETS minizip
@@ -66,4 +60,3 @@ if(FMILIB_INSTALL_SUBLIBS)
LIBRARY DESTINATION lib
)
endif()
-
diff --git a/Config.cmake/fmixml.cmake b/Config.cmake/fmixml.cmake
index dafb7ff..a93d757 100644
--- a/Config.cmake/fmixml.cmake
+++ b/Config.cmake/fmixml.cmake
@@ -83,7 +83,6 @@ endif()
include_directories("${FMIXMLDIR}/include" "${FMILIB_THIRDPARTYLIBS}/FMI/")
set(FMIXML_LIBRARIES fmixml)
-set(FMIXML_EXPAT_DIR "${FMILIB_THIRDPARTYLIBS}/Expat/expat-2.1.0")
set(FMIXMLHEADERS
include/FMI/fmi_xml_context.h
@@ -139,16 +138,7 @@ set(FMIXMLSOURCE
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DXML_STATIC -DFMI_XML_QUERY")
-### Add the options of expat to the cache and add it as subdirectory.
-set(BUILD_tools OFF CACHE BOOL "build the xmlwf tool for expat library")
-set(BUILD_examples OFF CACHE BOOL "build the examples for expat library")
-set(BUILD_tests OFF CACHE BOOL "build the tests for expat library")
-set(BUILD_shared OFF CACHE BOOL "build a shared expat library")
-add_subdirectory(ThirdParty/Expat/expat-2.1.0)
-
-set(EXPAT_INCLUDE_DIRS ThirdParty/Expat/expat-2.1.0/lib)
-
-include_directories("${EXPAT_INCLUDE_DIRS}" "${FMILIB_THIRDPARTYLIBS}/FMI/" "${FMIXMLGENDIR}/FMI1" "${FMIXMLGENDIR}/FMI2")
+include_directories("${FMILIB_THIRDPARTYLIBS}/FMI/" "${FMIXMLGENDIR}/FMI1" "${FMIXMLGENDIR}/FMI2")
PREFIXLIST(FMIXMLSOURCE ${FMIXMLDIR}/)
PREFIXLIST(FMIXMLHEADERS ${FMIXMLDIR}/)
@@ -164,6 +154,6 @@ debug_message(STATUS "adding fmixml")
add_library(fmixml ${FMILIBKIND} ${FMIXMLSOURCE} ${FMIXMLHEADERS})
-target_link_libraries(fmixml ${JMUTIL_LIBRARIES} expat)
+target_link_libraries(fmixml PRIVATE ${JMUTIL_LIBRARIES} PUBLIC CONAN_PKG::expat)
endif(NOT FMIXMLDIR)
diff --git a/Config.cmake/fmizip.cmake b/Config.cmake/fmizip.cmake
index 091fc4d..873c46d 100644
--- a/Config.cmake/fmizip.cmake
+++ b/Config.cmake/fmizip.cmake
@@ -22,7 +22,7 @@ if(NOT FMIZIPDIR)
add_subdirectory(Config.cmake/Minizip)
- include_directories("${FMIZIPDIR}/include" "${FMILIB_THIRDPARTYLIBS}/Minizip/minizip" "${FMILIB_THIRDPARTYLIBS}/FMI" "${FMILIB_THIRDPARTYLIBS}/Zlib/zlib-1.2.6" "${FMILibrary_BINARY_DIR}/zlib")
+ include_directories("${FMIZIPDIR}/include" "${FMILIB_THIRDPARTYLIBS}/Minizip/minizip" "${FMILIB_THIRDPARTYLIBS}/FMI")
set(FMIZIPSOURCE
${FMIZIPDIR}/src/fmi_zip_unzip.c@modelonrobinandersson - I would like to know if there is an update on this. Thank you.
Looks like the developers aren't very active right now. A different security issue (see #19) hasn't even been acknowledged yet.
@nknmsc Hi. We haven't looked into this more yet. To make this easier, can you share the minimal version of libexpat that is required to update to no longer be flagged by BlackDuck as a security issue?
Moreover, you are also able to contribute and make the desired changes, please see the README here.
@nknmsc I have created issue #22 to merge your report issue with another security issue reported in #19. I will close this one and we can take future discussion in #22. I will look into updating the thirdparty dependency libexpat by next month.
Also, do you happen to know what version of libexpat doesn't contain the security issue?
Expat 2.4.8 is now on master and there's a new tag with this fix in:
https://github.com/modelon-community/fmi-library/releases/tag/2.4.1
Thank you @filip-stenstrom and @modelonrobinandersson.