From 592ab485a70ab4c8e4cefc37bbdfb76110f9205e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Frings-F=C3=BCrst?= Date: Sun, 9 Jan 2022 18:59:51 +0100 Subject: New upstream version 0.9.6 --- .github/workflows/build-and-test.yml | 89 +++++++++++++++--- .gitignore | 2 + CMakeLists.txt | 57 ++++++++---- ChangeLog | 44 +++++++++ README.md | 5 +- THANKS | 2 + appveyor.yml | 18 +++- cmake/test_find_package/.gitignore | 2 + doc/preprocess.sh | 5 +- include/uriparser/Uri.h | 4 +- include/uriparser/UriBase.h | 2 +- liburiparser.pc.in | 4 +- make-distcheck.sh | 10 ++ src/UriNormalize.c | 53 ++++++----- src/UriParse.c | 18 ++-- test/MemoryManagerSuite.cpp | 175 ++++++++++++++++++++++++----------- test/test.cpp | 76 ++++++++++++++- tool/uriparse.c | 2 +- 18 files changed, 437 insertions(+), 131 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index ba5cb84..293c9ca 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -12,31 +12,70 @@ on: jobs: build_and_test: name: Build and test - runs-on: ubuntu-latest + strategy: + matrix: + include: + - name: Native Linux + cmake_args: >- + -DCMAKE_C_COMPILER=clang-13 + -DCMAKE_CXX_COMPILER=clang++-13 + cflags: >- + -fsanitize=address,undefined,leak + -fno-sanitize-recover=all + -fno-omit-frame-pointer + ldflags: >- + -fsanitize=address + - name: MingGW on Linux + cmake_args: >- + -DCMAKE_C_COMPILER=i686-w64-mingw32-gcc + -DCMAKE_CXX_COMPILER=i686-w64-mingw32-g++ + -DCMAKE_SYSTEM_NAME=Windows + -DWIN32=ON + -DMINGW=ON + runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v2.3.4 + - uses: actions/checkout@v2.4.0 - - name: Add Clang/LLVM repositories + - name: Add Clang/LLVM repositories (Non-MinGW) + if: "${{ ! contains(matrix.cmake_args, 'mingw') }}" run: |- wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add - - sudo add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-11 main' + sudo add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-13 main' - name: Install build dependencies run: |- + sudo apt-get update sudo apt-get install --yes --no-install-recommends -V \ - clang-11 \ cmake \ doxygen \ graphviz \ - llvm-11 \ lzip \ qhelpgenerator-qt5 \ qtchooser + - name: Install build dependencies (MinGW) + if: "${{ contains(matrix.cmake_args, 'mingw') }}" + run: |- + sudo dpkg --add-architecture i386 # for wine32 + sudo apt-add-repository ppa:ondrej/php -y # due to libwine:i386 conflicts + sudo apt-get update # again, due to new architecture + sudo apt-get install --yes --no-install-recommends -V \ + mingw-w64 \ + wine-stable \ + wine32 + + - name: Install build dependencies (Non-MinGW) + if: "${{ ! contains(matrix.cmake_args, 'mingw') }}" + run: |- + sudo apt-get install --yes --no-install-recommends -V \ + clang-13 \ + llvm-13 + - name: Build, test and install run: |- + set -x sed 's,:,\n,g' <<<"${PATH}" - clang --version + cmake --version GTEST_VERSION=1.8.1 GTEST_PREFIX=~/.local/ @@ -45,10 +84,15 @@ jobs: tar xf release-${GTEST_VERSION}.tar.gz ( cd googletest-release-${GTEST_VERSION}/ + + # Silence warning "Compatibility with CMake < 2.8.12 will be removed" + find -name CMakeLists.txt -print -exec sed 's/cmake_minimum_required.*/cmake_minimum_required(VERSION 3.0.2)/' -i {} \; + cmake \ -DBUILD_SHARED_LIBS=ON \ -DCVF_VERSION=${GTEST_VERSION} \ -DCMAKE_INSTALL_PREFIX:PATH=${GTEST_PREFIX} \ + ${{ matrix.cmake_args }} \ . make make install @@ -61,9 +105,7 @@ jobs: -O1 -g - -fsanitize=address,undefined,leak - -fno-sanitize-recover=all - -fno-omit-frame-pointer + ${{ matrix.cflags }} -Wall -Wextra @@ -71,7 +113,7 @@ jobs: ) CFLAGS="${compile_flags[*]} -std=c89" CXXFLAGS="${compile_flags[*]} -std=c++98" - LDFLAGS='-g -fsanitize=address' + LDFLAGS='-g ${{ matrix.ldflags }}' cmake_args=( -DCMAKE_INSTALL_PREFIX:PATH=${GTEST_PREFIX} @@ -80,20 +122,29 @@ jobs: -Wdeprecated -Werror=deprecated - -DCMAKE_C_COMPILER=clang-11 - -DCMAKE_CXX_COMPILER=clang++-11 -DCMAKE_C_FLAGS="${CFLAGS}" -DCMAKE_CXX_FLAGS="${CXXFLAGS}" -DCMAKE_EXE_LINKER_FLAGS="${LDFLAGS}" -DCMAKE_MODULE_LINKER_FLAGS="${LDFLAGS}" -DCMAKE_SHARED_LINKER_FLAGS="${LDFLAGS}" + ${{ matrix.cmake_args }} + -DURIPARSER_WARNINGS_AS_ERRORS=ON ) cmake "${cmake_args[@]}" -DCMAKE_INSTALL_INCLUDEDIR=include123 .. make VERBOSE=1 all + # NOTE: We need to copy some .dll files next to the + # Windows binaries so that they are ready to be executed + if [[ "${{ matrix.cmake_args }}" == *mingw* ]]; then + cp /usr/lib/gcc/i686-w64-mingw32/*-posix/libgcc_s_sjlj-1.dll ./ + cp /usr/lib/gcc/i686-w64-mingw32/*-posix/libstdc++-6.dll ./ + cp /usr/i686-w64-mingw32/lib/libwinpthread-1.dll ./ + cp "${GTEST_PREFIX}"/bin/libgtest.dll ./ + fi + make VERBOSE=1 test ARGS=--verbose cat Testing/Temporary/LastTest.log @@ -106,11 +157,19 @@ jobs: pushd cmake/test_find_package cmake "${cmake_args[@]}" . make VERBOSE=1 - ./hello + if [[ "${{ matrix.cmake_args }}" == *mingw* ]]; then + cp "${GTEST_PREFIX}"/bin/liburiparser-1.dll ./ + wine ./hello.exe + else + ./hello + fi popd git fetch --tags --unshallow origin # for "git describe" in make-distcheck.sh - ./make-distcheck.sh -DCMAKE_INSTALL_PREFIX:PATH=${GTEST_PREFIX} # without AddressSanitizer + GTEST_PREFIX="${GTEST_PREFIX}" \ + ./make-distcheck.sh \ + -DCMAKE_INSTALL_PREFIX:PATH=${GTEST_PREFIX} \ + ${{ matrix.cmake_args }} ! git status | fgrep -A100 'Untracked files:' # works best at the very end diff --git a/.gitignore b/.gitignore index d0bcb73..f7fe56a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,7 @@ +/.idea/ /config.h /build/ +/cmake-build-debug/ /CMakeCache.txt /CMakeDoxyfile.in /CMakeDoxygenDefaults.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 4b1ce44..9b80f71 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,14 +38,14 @@ cmake_minimum_required(VERSION 3.3) project(uriparser VERSION - 0.9.5 + 0.9.6 LANGUAGES C ) # See https://verbump.de/ for what these numbers do set(URIPARSER_SO_CURRENT 1) -set(URIPARSER_SO_REVISION 28) +set(URIPARSER_SO_REVISION 29) set(URIPARSER_SO_AGE 0) include(CheckCCompilerFlag) @@ -78,6 +78,11 @@ if(URIPARSER_BUILD_TOOLS AND NOT URIPARSER_BUILD_CHAR) message(SEND_ERROR "URIPARSER_BUILD_TOOLS=ON requires URIPARSER_BUILD_CHAR=ON.") endif() +if(URIPARSER_BUILD_TESTS) + # We have to call enable_language() before modifying any CMAKE_CXX_* variables + enable_language(CXX) +endif() + macro(uriparser_apply_msvc_runtime_to ref) string(REGEX REPLACE "/M[DT]d?" ${URIPARSER_MSVC_RUNTIME} ${ref} "${${ref}}") endmacro() @@ -104,17 +109,6 @@ if(URIPARSER_COMPILER_SUPPORTS_VISIBILITY) set(URIPARSER_EXTRA_COMPILE_FLAGS "${URIPARSER_EXTRA_COMPILE_FLAGS} -fvisibility=hidden") endif() -if(URIPARSER_WARNINGS_AS_ERRORS) - if(MSVC) - add_definitions(/WX) - else() - set(URIPARSER_EXTRA_COMPILE_FLAGS "${URIPARSER_EXTRA_COMPILE_FLAGS} -Werror") - endif() -endif() - -set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${URIPARSER_EXTRA_COMPILE_FLAGS}") -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${URIPARSER_EXTRA_COMPILE_FLAGS}") - # # config.h # @@ -259,8 +253,6 @@ endif() # C++ test runner # if(URIPARSER_BUILD_TESTS) - enable_language(CXX) - if(MSVC AND URIPARSER_MSVC_RUNTIME) uriparser_apply_msvc_runtime_to(CMAKE_CXX_FLAGS) uriparser_apply_msvc_runtime_to(CMAKE_CXX_FLAGS_DEBUG) @@ -286,6 +278,7 @@ if(URIPARSER_BUILD_TESTS) target_compile_definitions(testrunner PRIVATE URI_STATIC_BUILD) if(MSVC) + target_compile_definitions(testrunner PRIVATE -D_CRT_NONSTDC_NO_WARNINGS) target_compile_definitions(testrunner PRIVATE -D_CRT_SECURE_NO_WARNINGS) endif() @@ -311,16 +304,36 @@ if(URIPARSER_BUILD_TESTS) target_compile_options(testrunner PRIVATE /EHsc) endif() + if(MINGW) + set(_URIPARSER_TEST_COMMAND wine testrunner) + else() + set(_URIPARSER_TEST_COMMAND testrunner) + endif() + add_test( NAME test COMMAND - testrunner + ${_URIPARSER_TEST_COMMAND} ) add_custom_target(check COMMAND ${CMAKE_CTEST_COMMAND}) endif() +# +# Compiler flags +# +if(URIPARSER_WARNINGS_AS_ERRORS) + if(MSVC) + add_definitions(/WX) + else() + set(URIPARSER_EXTRA_COMPILE_FLAGS "${URIPARSER_EXTRA_COMPILE_FLAGS} -Werror") + endif() +endif() + +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${URIPARSER_EXTRA_COMPILE_FLAGS}") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${URIPARSER_EXTRA_COMPILE_FLAGS}") + # # Doxygen API documentation # @@ -423,6 +436,18 @@ uriparser_install( # pkg-config file # if(NOT MSVC) + if(CMAKE_INSTALL_LIBDIR MATCHES "^/") + set(_URIPARSER_PKGCONFIG_LIBDIR "${CMAKE_INSTALL_LIBDIR}") + else() + set(_URIPARSER_PKGCONFIG_LIBDIR "\${exec_prefix}/${CMAKE_INSTALL_LIBDIR}") + endif() + + if(CMAKE_INSTALL_INCLUDEDIR MATCHES "^/") + set(_URIPARSER_PKGCONFIG_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}") + else() + set(_URIPARSER_PKGCONFIG_INCLUDEDIR "\${prefix}/${CMAKE_INSTALL_INCLUDEDIR}") + endif() + configure_file(${CMAKE_CURRENT_SOURCE_DIR}/liburiparser.pc.in liburiparser.pc @ONLY) uriparser_install( FILES diff --git a/ChangeLog b/ChangeLog index a3d7844..cf1a837 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,50 @@ NOTE: uriparser is looking for help with a few things: https://github.com/uriparser/uriparser/labels/help%20wanted If you can help, please get in touch. Thanks! +2022-01-06 -- 0.9.6 + +>>>>>>>>>>>>> SECURITY >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + * Fixed: [CVE-2021-46141] + Fix a bug affecting both uriNormalizeSyntax* and uriMakeOwner* + functions where the text range in .hostText would not be duped using + malloc but remain unchanged (and hence "not owned") for URIs with + an IPv4 or IPv6 address hostname; depending on how an application + uses uriparser, this could lead the application into a use-after-free + situation. + As the second half, fix uriFreeUriMembers* functions that would not + free .hostText memory for URIs with an IPv4 or IPv6 address host; + also, calling uriFreeUriMembers* multiple times on a URI of this + very nature would result in trying to free pointers to stack + (rather than heap) memory (GitHub #121, GitHub #124) + Commit 987b046e41f407d17c622e580fc82a5e834b4329 + Commit b1a34743bc1472e055d886e29e9b53f670eb3282 + * Fixed: [CVE-2021-46142] + Fix functions uriNormalizeSyntax* for out-of-memory situations + (i.e. malloc returning NULL) for URIs containing empty segments + (any of user info, host text, query, or fragment) where previously + pointers to stack (rather than heap) memory were freed (GitHub #122, + GitHub #124) + Commit c0483990e6b5b454f7c8752b36760cfcb0d093f5 +>>>>>>>>>>>>> SECURITY >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + * Fixed: CMake: Call "enable_language(CXX)" prior to tinkering with + CMAKE_CXX_* variables (GitHub #110) + Thanks to Alexander Richardson for the patch (originally at libexpat) + * Fixed: CMake: Support absolute paths for both CMAKE_INSTALL_LIBDIR + and CMAKE_INSTALL_INCLUDEDIR (GitHub #114) + Thanks to Rafael Fontenelle for bringing this up (originally at libexpat) + * Fixed: Windows: Address MSVC compiler warnings (GitHub #111, GitHub #113) + * Fixed: Documentation: Space requirements for uriUriStringToUnixFilename + did not take into account short form "file:/bin/bash" of RFC 8089 of 2017 + (with prefix "file:/" rather than "file:///") that uriparser supports + since release 0.8.6 in 2018 (GitHub #118, GitHub #119) + * Fixed: Compile error with MinGW GCC 9 related to a mismatched prototype + for function inet_ntop (GitHub #117, GitHub #120) + Thanks to Sandro Mani for the report! + * Fixed: Compile warnings in test suite code (GitHub #120) + * Improved: Respect variable ${CPP} in doc/preprocess.sh (GitHub #115) + * Added: Test suite invocation for MinGW using Wine (GitHub #120) + * Soname: 1:29:0 — see https://verbump.de/ for what these numbers do + 2021-03-18 -- 0.9.5 * Fixed: Fix a bug regarding section "5.2.4. Remove Dot Segments" diff --git a/README.md b/README.md index 3f92bfc..2846e9d 100644 --- a/README.md +++ b/README.md @@ -18,8 +18,11 @@ please check out [https://uriparser.github.io/](https://uriparser.github.io/). # Example use from an existing CMake project + ```cmake -project(hello VERSION 1.0) +cmake_minimum_required(VERSION 3.3) + +project(hello VERSION 1.0.0) find_package(uriparser 0.9.2 CONFIG REQUIRED char wchar_t) diff --git a/THANKS b/THANKS index 90e371b..c696f93 100644 --- a/THANKS +++ b/THANKS @@ -3,6 +3,7 @@ Adam Gross Adeodato Simó Adrian Manrique Alexander Klink +Alexander Richardson Arkadiusz Miskiewicz Andreas Sturmlechner Blair Sadewitz @@ -44,6 +45,7 @@ myd7349 Periklis Akritidis Philip de Nier Radu Hociung +Rafael Fontenelle Ralf S. Engelschall Rakesh Pandit René Rebe diff --git a/appveyor.yml b/appveyor.yml index 7e36cdb..ec111fa 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -41,7 +41,7 @@ configuration: # - Release environment: - GTEST_VERSION: 1.8.1 + GTEST_VERSION: 1.10.0 # https://www.appveyor.com/docs/windows-images-software/ matrix: @@ -65,6 +65,16 @@ environment: CMAKE_GENERATOR: Visual Studio 15 2017 Win64 PLATFORM: x64 + # Visual Studio 2019, 32 bit + - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019 + CMAKE_GENERATOR: Visual Studio 16 2019 + COMMON_CMAKE_ARGS: -A Win32 + + # Visual Studio 2019, 64 bit + - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019 + CMAKE_GENERATOR: Visual Studio 16 2019 + COMMON_CMAKE_ARGS: -A x64 + before_build: - curl -fsSL -o release-%GTEST_VERSION%.zip https://github.com/google/googletest/archive/release-%GTEST_VERSION%.zip - unzip -q release-%GTEST_VERSION%.zip @@ -72,12 +82,13 @@ before_build: - cmake -G "%CMAKE_GENERATOR%" -DCVF_VERSION=%GTEST_VERSION% + %COMMON_CMAKE_ARGS% . - cmake --build . --config Release -- /m # BEGIN Enrich folder to make FindGTest.cmake happy - md googletest\lib - - copy googlemock\gtest\Release\gtest.lib googletest\lib - - copy googlemock\gtest\Release\gtest_main.lib googletest\lib + - copy lib\Release\gtest.lib googletest\lib + - copy lib\Release\gtest_main.lib googletest\lib # END - cd .. - mkdir build @@ -89,6 +100,7 @@ before_build: -DURIPARSER_BUILD_DOCS=OFF -DURIPARSER_MSVC_RUNTIME=/MT -DURIPARSER_WARNINGS_AS_ERRORS=ON + %COMMON_CMAKE_ARGS% .. build: diff --git a/cmake/test_find_package/.gitignore b/cmake/test_find_package/.gitignore index e41efe9..61ec265 100644 --- a/cmake/test_find_package/.gitignore +++ b/cmake/test_find_package/.gitignore @@ -3,3 +3,5 @@ /Makefile /cmake_install.cmake /hello +/hello.exe +/liburiparser-1.dll diff --git a/doc/preprocess.sh b/doc/preprocess.sh index a663cd1..c77bb6a 100755 --- a/doc/preprocess.sh +++ b/doc/preprocess.sh @@ -1,3 +1,4 @@ -#! /bin/sh +#! /usr/bin/env bash # Run GCC preprocessor and delete empty lines -cpp -DURI_DOXYGEN -DURI_NO_UNICODE -C -I ../include $1 | sed -e '/^$/d' -e 's/COMMENT_HACK//g' +: ${CPP:=cpp} +"${CPP}" -DURI_DOXYGEN -DURI_NO_UNICODE -C -I ../include "$1" | sed -e '/^$/d' -e 's/COMMENT_HACK//g' diff --git a/include/uriparser/Uri.h b/include/uriparser/Uri.h index 44d8760..ea4a913 100644 --- a/include/uriparser/Uri.h +++ b/include/uriparser/Uri.h @@ -1,4 +1,4 @@ -/* 6db8b5726a796167bb96b3d83ff9ac6792a01474dbe3778deb3c2a25d60b3693 (0.9.5+) +/* 0beb20dace01f4b2fa24c3bf9f7e8e616f621e4697260805a2b63a4d190e64c0 (0.9.6+) * * uriparser - RFC 3986 URI parsing library * @@ -769,7 +769,7 @@ URI_PUBLIC int URI_FUNC(WindowsFilenameToUriString)(const URI_CHAR * filename, /** * Extracts a Unix filename from a %URI string. - * The destination buffer must be large enough to hold len(uriString) + 1 - 7 + * The destination buffer must be large enough to hold len(uriString) + 1 - 5 * characters in case of an absolute %URI or len(uriString) + 1 in case * of a relative %URI. * diff --git a/include/uriparser/UriBase.h b/include/uriparser/UriBase.h index 565abcf..089c2c6 100644 --- a/include/uriparser/UriBase.h +++ b/include/uriparser/UriBase.h @@ -55,7 +55,7 @@ /* Version */ #define URI_VER_MAJOR 0 #define URI_VER_MINOR 9 -#define URI_VER_RELEASE 5 +#define URI_VER_RELEASE 6 #define URI_VER_SUFFIX_ANSI "" #define URI_VER_SUFFIX_UNICODE URI_ANSI_TO_UNICODE(URI_VER_SUFFIX_ANSI) diff --git a/liburiparser.pc.in b/liburiparser.pc.in index 9547860..cb78000 100644 --- a/liburiparser.pc.in +++ b/liburiparser.pc.in @@ -1,7 +1,7 @@ prefix=@CMAKE_INSTALL_PREFIX@ exec_prefix=${prefix} -libdir=${exec_prefix}/@CMAKE_INSTALL_LIBDIR@ -includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@ +libdir=@_URIPARSER_PKGCONFIG_LIBDIR@ +includedir=@_URIPARSER_PKGCONFIG_INCLUDEDIR@ Name: liburiparser Description: URI parsing and handling library diff --git a/make-distcheck.sh b/make-distcheck.sh index 0918a08..2968dd0 100755 --- a/make-distcheck.sh +++ b/make-distcheck.sh @@ -72,6 +72,16 @@ check_tarball() ( cmake "${@:2}" .. make + + # NOTE: We need to copy some .dll files next to the + # Windows binaries so that they are ready to be executed + if [[ "${*:2}" == *mingw* ]]; then + cp /usr/lib/gcc/i686-w64-mingw32/*-posix/libgcc_s_sjlj-1.dll ./ + cp /usr/lib/gcc/i686-w64-mingw32/*-posix/libstdc++-6.dll ./ + cp /usr/i686-w64-mingw32/lib/libwinpthread-1.dll ./ + cp "${GTEST_PREFIX:?}"/bin/libgtest.dll ./ + fi + make test make DESTDIR="${PWD}"/ROOT/ install ) diff --git a/src/UriNormalize.c b/src/UriNormalize.c index 04a8e5f..eb072b2 100644 --- a/src/UriNormalize.c +++ b/src/UriNormalize.c @@ -115,13 +115,17 @@ static void URI_FUNC(PreventLeakage)(URI_TYPE(Uri) * uri, static URI_INLINE void URI_FUNC(PreventLeakage)(URI_TYPE(Uri) * uri, unsigned int revertMask, UriMemoryManager * memory) { if (revertMask & URI_NORMALIZE_SCHEME) { + /* NOTE: A scheme cannot be the empty string + * so no need to compare .first with .afterLast, here. */ memory->free(memory, (URI_CHAR *)uri->scheme.first); uri->scheme.first = NULL; uri->scheme.afterLast = NULL; } if (revertMask & URI_NORMALIZE_USER_INFO) { - memory->free(memory, (URI_CHAR *)uri->userInfo.first); + if (uri->userInfo.first != uri->userInfo.afterLast) { + memory->free(memory, (URI_CHAR *)uri->userInfo.first); + } uri->userInfo.first = NULL; uri->userInfo.afterLast = NULL; } @@ -129,16 +133,18 @@ static URI_INLINE void URI_FUNC(PreventLeakage)(URI_TYPE(Uri) * uri, if (revertMask & URI_NORMALIZE_HOST) { if (uri->hostData.ipFuture.first != NULL) { /* IPvFuture */ + /* NOTE: An IPvFuture address cannot be the empty string + * so no need to compare .first with .afterLast, here. */ memory->free(memory, (URI_CHAR *)uri->hostData.ipFuture.first); uri->hostData.ipFuture.first = NULL; uri->hostData.ipFuture.afterLast = NULL; uri->hostText.first = NULL; uri->hostText.afterLast = NULL; - } else if ((uri->hostText.first != NULL) - && (uri->hostData.ip4 == NULL) - && (uri->hostData.ip6 == NULL)) { + } else if (uri->hostText.first != NULL) { /* Regname */ - memory->free(memory, (URI_CHAR *)uri->hostText.first); + if (uri->hostText.first != uri->hostText.afterLast) { + memory->free(memory, (URI_CHAR *)uri->hostText.first); + } uri->hostText.first = NULL; uri->hostText.afterLast = NULL; } @@ -161,13 +167,17 @@ static URI_INLINE void URI_FUNC(PreventLeakage)(URI_TYPE(Uri) * uri, } if (revertMask & URI_NORMALIZE_QUERY) { - memory->free(memory, (URI_CHAR *)uri->query.first); + if (uri->query.first != uri->query.afterLast) { + memory->free(memory, (URI_CHAR *)uri->query.first); + } uri->query.first = NULL; uri->query.afterLast = NULL; } if (revertMask & URI_NORMALIZE_FRAGMENT) { - memory->free(memory, (URI_CHAR *)uri->fragment.first); + if (uri->fragment.first != uri->fragment.afterLast) { + memory->free(memory, (URI_CHAR *)uri->fragment.first); + } uri->fragment.first = NULL; uri->fragment.afterLast = NULL; } @@ -407,22 +417,19 @@ static URI_INLINE UriBool URI_FUNC(MakeOwnerEngine)(URI_TYPE(Uri) * uri, /* Host */ if ((*doneMask & URI_NORMALIZE_HOST) == 0) { - if ((uri->hostData.ip4 == NULL) - && (uri->hostData.ip6 == NULL)) { - if (uri->hostData.ipFuture.first != NULL) { - /* IPvFuture */ - if (!URI_FUNC(MakeRangeOwner)(doneMask, URI_NORMALIZE_HOST, - &(uri->hostData.ipFuture), memory)) { - return URI_FALSE; /* Raises malloc error */ - } - uri->hostText.first = uri->hostData.ipFuture.first; - uri->hostText.afterLast = uri->hostData.ipFuture.afterLast; - } else if (uri->hostText.first != NULL) { - /* Regname */ - if (!URI_FUNC(MakeRangeOwner)(doneMask, URI_NORMALIZE_HOST, - &(uri->hostText), memory)) { - return URI_FALSE; /* Raises malloc error */ - } + if (uri->hostData.ipFuture.first != NULL) { + /* IPvFuture */ + if (!URI_FUNC(MakeRangeOwner)(doneMask, URI_NORMALIZE_HOST, + &(uri->hostData.ipFuture), memory)) { + return URI_FALSE; /* Raises malloc error */ + } + uri->hostText.first = uri->hostData.ipFuture.first; + uri->hostText.afterLast = uri->hostData.ipFuture.afterLast; + } else if (uri->hostText.first != NULL) { + /* Regname */ + if (!URI_FUNC(MakeRangeOwner)(doneMask, URI_NORMALIZE_HOST, + &(uri->hostText), memory)) { + return URI_FALSE; /* Raises malloc error */ } } } diff --git a/src/UriParse.c b/src/UriParse.c index 0bc8f0a..eefa8d1 100644 --- a/src/UriParse.c +++ b/src/UriParse.c @@ -2287,22 +2287,24 @@ int URI_FUNC(FreeUriMembersMm)(URI_TYPE(Uri) * uri, UriMemoryManager * memory) { uri->userInfo.afterLast = NULL; } - /* Host data - IPvFuture */ + /* Host data - IPvFuture (may affect host text) */ if (uri->hostData.ipFuture.first != NULL) { + /* NOTE: .hostData.ipFuture may hold the very same range pointers + * as .hostText; then we need to prevent freeing memory twice. */ + if (uri->hostText.first == uri->hostData.ipFuture.first) { + uri->hostText.first = NULL; + uri->hostText.afterLast = NULL; + } + if (uri->hostData.ipFuture.first != uri->hostData.ipFuture.afterLast) { memory->free(memory, (URI_CHAR *)uri->hostData.ipFuture.first); } uri->hostData.ipFuture.first = NULL; uri->hostData.ipFuture.afterLast = NULL; - uri->hostText.first = NULL; - uri->hostText.afterLast = NULL; } - /* Host text (if regname, after IPvFuture!) */ - if ((uri->hostText.first != NULL) - && (uri->hostData.ip4 == NULL) - && (uri->hostData.ip6 == NULL)) { - /* Real regname */ + /* Host text (after IPvFuture, see above) */ + if (uri->hostText.first != NULL) { if (uri->hostText.first != uri->hostText.afterLast) { memory->free(memory, (URI_CHAR *)uri->hostText.first); } diff --git a/test/MemoryManagerSuite.cpp b/test/MemoryManagerSuite.cpp index a828d76..e4af977 100644 --- a/test/MemoryManagerSuite.cpp +++ b/test/MemoryManagerSuite.cpp @@ -36,76 +36,99 @@ extern "C" { namespace { -class CallCountLog { -public: - unsigned int callCountFree; - CallCountLog() : callCountFree(0) { - // no-op - } -}; +static void * failingMalloc(UriMemoryManager * memory, size_t size); +static void * failingCalloc(UriMemoryManager * memory, size_t nmemb, size_t size); +static void * failingRealloc(UriMemoryManager * memory, void * ptr, size_t size); +static void * failingReallocarray(UriMemoryManager * memory, void * ptr, size_t nmemb, size_t size); +static void countingFree(UriMemoryManager * memory, void * ptr); -static void * failingMalloc(UriMemoryManager * URI_UNUSED(memory), - size_t URI_UNUSED(size)) { - return NULL; -} +class FailingMemoryManager { +private: + UriMemoryManager memoryManager; + unsigned int callCountAlloc; + unsigned int callCountFree; + unsigned int failAllocAfterTimes; + friend void * failingMalloc(UriMemoryManager * memory, size_t size); + friend void * failingCalloc(UriMemoryManager * memory, size_t nmemb, size_t size); + friend void * failingRealloc(UriMemoryManager * memory, void * ptr, size_t size); + friend void * failingReallocarray(UriMemoryManager * memory, void * ptr, size_t nmemb, size_t size); + friend void countingFree(UriMemoryManager * memory, void * ptr); -static void * failingCalloc(UriMemoryManager * URI_UNUSED(memory), - size_t URI_UNUSED(nmemb), size_t URI_UNUSED(size)) { - return NULL; -} +public: + FailingMemoryManager(unsigned int failAllocAfterTimes = 0) + : callCountAlloc(0), callCountFree(0), + failAllocAfterTimes(failAllocAfterTimes) { + this->memoryManager.malloc = failingMalloc; + this->memoryManager.calloc = failingCalloc; + this->memoryManager.realloc = failingRealloc; + this->memoryManager.reallocarray = failingReallocarray; + this->memoryManager.free = countingFree; + this->memoryManager.userData = this; + } + UriMemoryManager * operator&() { + return &(this->memoryManager); + } + unsigned int getCallCountFree() const { + return this->callCountFree; + } +}; -static void * failingRealloc(UriMemoryManager * URI_UNUSED(memory), - void * URI_UNUSED(ptr), size_t URI_UNUSED(size)) { - return NULL; + + +static void * failingMalloc(UriMemoryManager * memory, size_t size) { + FailingMemoryManager * const fmm = static_cast(memory->userData); + fmm->callCountAlloc++; + if (fmm->callCountAlloc > fmm->failAllocAfterTimes) { + errno = ENOMEM; + return NULL; + } + return malloc(size); } -static void * failingReallocarray(UriMemoryManager * URI_UNUSED(memory), - void * URI_UNUSED(ptr), size_t URI_UNUSED(nmemb), - size_t URI_UNUSED(size)) { - return NULL; +static void * failingCalloc(UriMemoryManager * memory, size_t nmemb, size_t size) { + FailingMemoryManager * const fmm = static_cast(memory->userData); + fmm->callCountAlloc++; + if (fmm->callCountAlloc > fmm->failAllocAfterTimes) { + errno = ENOMEM; + return NULL; + } + return calloc(nmemb, size); } -static void countingFree(UriMemoryManager * memory, void * ptr) { - static_cast(memory->userData)->callCountFree++; - free(ptr); +static void * failingRealloc(UriMemoryManager * memory, void * ptr, size_t size) { + FailingMemoryManager * const fmm = static_cast(memory->userData); + fmm->callCountAlloc++; + if (fmm->callCountAlloc > fmm->failAllocAfterTimes) { + errno = ENOMEM; + return NULL; + } + return realloc(ptr, size); } -class FailingMemoryManager { -private: - UriMemoryManager memoryManager; - CallCountLog callCountLog; +static void * failingReallocarray(UriMemoryManager * memory, void * ptr, size_t nmemb, size_t size) { + return uriEmulateReallocarray(memory, ptr, nmemb, size); +} -public: - FailingMemoryManager() { - this->memoryManager.malloc = failingMalloc; - this->memoryManager.calloc = failingCalloc; - this->memoryManager.realloc = failingRealloc; - this->memoryManager.reallocarray = failingReallocarray; - this->memoryManager.free = countingFree; - this->memoryManager.userData = &(this->callCountLog); - } - UriMemoryManager * operator&() { - return &(this->memoryManager); - } - unsigned int getCallCountFree() const { - return this->callCountLog.callCountFree; - } -}; +static void countingFree(UriMemoryManager * memory, void * ptr) { + FailingMemoryManager * const fmm = static_cast(memory->userData); + fmm->callCountFree++; + return free(ptr); +} @@ -318,11 +341,11 @@ TEST(FailingMemoryManagerSuite, DissectQueryMallocExMm) { TEST(FailingMemoryManagerSuite, FreeQueryListMm) { UriQueryListA * const queryList = parseQueryList("k1=v1"); FailingMemoryManager failingMemoryManager; - ASSERT_EQ(failingMemoryManager.getCallCountFree(), 0); + ASSERT_EQ(failingMemoryManager.getCallCountFree(), 0U); uriFreeQueryListMmA(queryList, &failingMemoryManager); - ASSERT_GE(failingMemoryManager.getCallCountFree(), 1); + ASSERT_GE(failingMemoryManager.getCallCountFree(), 1U); } @@ -330,25 +353,65 @@ TEST(FailingMemoryManagerSuite, FreeQueryListMm) { TEST(FailingMemoryManagerSuite, FreeUriMembersMm) { UriUriA uri = parse("http://example.org/"); FailingMemoryManager failingMemoryManager; - ASSERT_EQ(failingMemoryManager.getCallCountFree(), 0); + ASSERT_EQ(failingMemoryManager.getCallCountFree(), 0U); uriFreeUriMembersMmA(&uri, &failingMemoryManager); - ASSERT_GE(failingMemoryManager.getCallCountFree(), 1); + ASSERT_GE(failingMemoryManager.getCallCountFree(), 1U); uriFreeUriMembersA(&uri); } +namespace { + void testNormalizeSyntaxWithFailingMallocCallsFreeTimes(const char * uriString, + unsigned int mask, + unsigned int failAllocAfterTimes = 0, + unsigned int expectedCallCountFree = 0) { + UriUriA uri = parse(uriString); + FailingMemoryManager failingMemoryManager(failAllocAfterTimes); + ASSERT_EQ(uriNormalizeSyntaxExMmA(&uri, mask, &failingMemoryManager), + URI_ERROR_MALLOC); -TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMm) { - UriUriA uri = parse("hTTp://example.org/path"); - const unsigned int mask = URI_NORMALIZE_SCHEME; // anything but URI_NORMALIZED - FailingMemoryManager failingMemoryManager; + EXPECT_EQ(failingMemoryManager.getCallCountFree(), expectedCallCountFree); - ASSERT_EQ(uriNormalizeSyntaxExMmA(&uri, mask, &failingMemoryManager), - URI_ERROR_MALLOC); + uriFreeUriMembersA(&uri); + } +} // namespace - uriFreeUriMembersA(&uri); +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmScheme) { + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("hTTp://example.org/path", URI_NORMALIZE_SCHEME); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmEmptyUserInfo) { + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//@:123", URI_NORMALIZE_USER_INFO); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmEmptyHostRegname) { + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//:123", URI_NORMALIZE_HOST); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmEmptyQuery) { + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//:123?", URI_NORMALIZE_QUERY); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmEmptyFragment) { + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//:123#", URI_NORMALIZE_FRAGMENT); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmHostTextIp4) { // issue #121 + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//192.0.2.0:123" /* RFC 5737 */, URI_NORMALIZE_HOST, 1, 1); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmHostTextIp6) { // issue #121 + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//[2001:db8::]:123" /* RFC 3849 */, URI_NORMALIZE_HOST, 1, 1); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmHostTextRegname) { // issue #121 + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//host123.test:123" /* RFC 6761 */, URI_NORMALIZE_HOST, 1, 1); +} + +TEST(FailingMemoryManagerSuite, NormalizeSyntaxExMmHostTextFuture) { // issue #121 + testNormalizeSyntaxWithFailingMallocCallsFreeTimes("//[v7.X]:123" /* arbitrary IPvFuture */, URI_NORMALIZE_HOST, 1, 1); } diff --git a/test/test.cpp b/test/test.cpp index 4b156a4..31e9866 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -267,7 +267,7 @@ TEST(UriSuite, TestIpSixOverread) { // NOTE: This string is designed to not have a terminator char uriText[2 + 3 + 2 + 1 + 1]; - strncpy(uriText, "//[::44.1", sizeof(uriText)); + memcpy(uriText, "//[::44.1", sizeof(uriText)); EXPECT_EQ(uriParseSingleUriExA(&uri, uriText, uriText + sizeof(uriText), &errorPos), URI_ERROR_SYNTAX); @@ -2216,6 +2216,45 @@ TEST(FreeUriMembersSuite, MultiFreeWorksFine) { uriFreeUriMembersA(&uri); // second time } +namespace { + void testFreeUriMembersFreesHostText(const char *const uriFirst) { // issue #121 + const char *const uriAfterLast = uriFirst + strlen(uriFirst); + UriUriA uri; + + EXPECT_EQ(uriParseSingleUriA(&uri, uriFirst, NULL), URI_SUCCESS); + EXPECT_EQ(uriMakeOwnerA(&uri), URI_SUCCESS); + + EXPECT_EQ(uri.owner, URI_TRUE); + EXPECT_TRUE(uri.hostText.first); + EXPECT_TRUE(uri.hostText.afterLast); + EXPECT_NE(uri.hostText.first, uri.hostText.afterLast); + URI_EXPECT_RANGE_OUTSIDE(uri.hostText, uriFirst, uriAfterLast); + + uriFreeUriMembersA(&uri); + + EXPECT_FALSE(uri.hostText.first); + EXPECT_FALSE(uri.hostText.afterLast); + + uriFreeUriMembersA(&uri); // second time + } +} // namespace + +TEST(FreeUriMembersSuite, FreeUriMembersFreesHostTextIp4) { // issue #121 + testFreeUriMembersFreesHostText("//192.0.2.0"); // RFC 5737 +} + +TEST(FreeUriMembersSuite, FreeUriMembersFreesHostTextIp6) { // issue #121 + testFreeUriMembersFreesHostText("//[2001:db8::]"); // RFC 3849 +} + +TEST(FreeUriMembersSuite, FreeUriMembersFreesHostTextRegname) { // issue #121 + testFreeUriMembersFreesHostText("//host123.test"); // RFC 6761 +} + +TEST(FreeUriMembersSuite, FreeUriMembersFreesHostTextFuture) { // issue #121 + testFreeUriMembersFreesHostText("//[v7.X]"); // arbitrary IPvFuture +} + TEST(MakeOwnerSuite, MakeOwner) { const char * const uriString = "scheme://user:pass@[v7.X]:55555/path/../path/?query#fragment"; UriUriA uri; @@ -2275,6 +2314,41 @@ TEST(MakeOwnerSuite, MakeOwner) { uriFreeUriMembersA(&uri); } +namespace { + void testMakeOwnerCopiesHostText(const char *const uriFirst) { // issue #121 + const char *const uriAfterLast = uriFirst + strlen(uriFirst); + UriUriA uri; + + EXPECT_EQ(uriParseSingleUriA(&uri, uriFirst, NULL), URI_SUCCESS); + EXPECT_EQ(uri.owner, URI_FALSE); + URI_EXPECT_RANGE_BETWEEN(uri.hostText, uriFirst, uriAfterLast); + + EXPECT_EQ(uriMakeOwnerA(&uri), URI_SUCCESS); + + EXPECT_EQ(uri.owner, URI_TRUE); + URI_EXPECT_RANGE_OUTSIDE(uri.hostText, uriFirst, uriAfterLast); + + uriFreeUriMembersA(&uri); + uriFreeUriMembersA(&uri); // tried freeing stack pointers before the fix + } +} // namespace + +TEST(MakeOwnerSuite, MakeOwnerCopiesHostTextIp4) { // issue #121 + testMakeOwnerCopiesHostText("//192.0.2.0"); // RFC 5737 +} + +TEST(MakeOwnerSuite, MakeOwnerCopiesHostTextIp6) { // issue #121 + testMakeOwnerCopiesHostText("//[2001:db8::]"); // RFC 3849 +} + +TEST(MakeOwnerSuite, MakeOwnerCopiesHostTextRegname) { // issue #121 + testMakeOwnerCopiesHostText("//host123.test"); // RFC 6761 +} + +TEST(MakeOwnerSuite, MakeOwnerCopiesHostTextFuture) { // issue #121 + testMakeOwnerCopiesHostText("//[v7.X]"); // arbitrary IPvFuture +} + TEST(ParseIpFourAddressSuite, FourSaneOctets) { unsigned char octetOutput[4]; const char * const ipAddressText = "111.22.3.40"; diff --git a/tool/uriparse.c b/tool/uriparse.c index 421f99a..027f2d2 100644 --- a/tool/uriparse.c +++ b/tool/uriparse.c @@ -44,7 +44,7 @@ # include # include # ifdef __MINGW32__ -WINSOCK_API_LINKAGE const char WSAAPI inet_ntop( +WINSOCK_API_LINKAGE const char * WSAAPI inet_ntop( int af, const void *src, char *dst, socklen_t size); # endif #else -- cgit v1.2.3