Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libexpat 2.1.0 flagged as a security risk by blackduck #18

Closed
nknmsc opened this issue Mar 21, 2022 · 12 comments
Closed

libexpat 2.1.0 flagged as a security risk by blackduck #18

nknmsc opened this issue Mar 21, 2022 · 12 comments

Comments

@nknmsc
Copy link

nknmsc commented Mar 21, 2022

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 [email protected] but have not heard anything back. Hence I am posting this here.

Please help. Thank you.

@modelonrobinandersson
Copy link
Collaborator

modelonrobinandersson commented Mar 28, 2022

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 [email protected] 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.

@slietzau
Copy link

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).

@nknmsc
Copy link
Author

nknmsc commented May 2, 2022

@modelonrobinandersson - I would like to know if there is an update on this. Thank you.

@chria
Copy link
Collaborator

chria commented May 5, 2022

Hi, I'm afraid that there is no update on this at the moment.

@chria
Copy link
Collaborator

chria commented May 6, 2022

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.

@stefansli
Copy link

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

@nknmsc
Copy link
Author

nknmsc commented Jul 28, 2022

@modelonrobinandersson - I would like to know if there is an update on this. Thank you.

@stefansli
Copy link

stefansli commented Jul 28, 2022

Looks like the developers aren't very active right now. A different security issue (see #19) hasn't even been acknowledged yet.

@modelonrobinandersson
Copy link
Collaborator

@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.

@modelonrobinandersson
Copy link
Collaborator

@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?

@filip-stenstrom
Copy link
Collaborator

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

@nknmsc
Copy link
Author

nknmsc commented Sep 15, 2022

Thank you @filip-stenstrom and @modelonrobinandersson.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants