Skip to content
Permalink
Browse files Browse the repository at this point in the history
CVE-2013-6401: Change hash function, randomize hashes
Thanks to Florian Weimer and Eric Sesterhenn for reporting, reviewing
and testing.
  • Loading branch information
akheron committed Feb 11, 2014
1 parent b9c588d commit 8f80c2d
Show file tree
Hide file tree
Showing 19 changed files with 873 additions and 122 deletions.
49 changes: 39 additions & 10 deletions CMakeLists.txt
Expand Up @@ -52,6 +52,8 @@ project (jansson C)

# Options
OPTION (BUILD_SHARED_LIBS "Build shared libraries." OFF)
OPTION (USE_URANDOM "Use /dev/urandom to seed the hash function." ON)
OPTION (USE_WINDOWS_CRYPTOAPI "Use CryptGenRandom to seed the hash function." ON)

if (MSVC)
# This option must match the settings used in your program, in particular if you
Expand Down Expand Up @@ -85,12 +87,12 @@ set (JANSSON_SOVERSION 4)
# for CheckFunctionKeywords
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

INCLUDE (CheckCSourceCompiles)
include (CheckFunctionExists)
include (CheckFunctionKeywords)
include (CheckIncludeFiles)
include (CheckTypeSize)


if (MSVC)
# Turn off Microsofts "security" warnings.
add_definitions( "/W3 /D_CRT_SECURE_NO_WARNINGS /wd4005 /wd4996 /nologo" )
Expand All @@ -106,14 +108,25 @@ if (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX)
set(CMAKE_C_FLAGS "-fPIC")
endif()


check_include_files (endian.h HAVE_ENDIAN_H)
check_include_files (fcntl.h HAVE_FCNTL_H)
check_include_files (sched.h HAVE_SCHED_H)
check_include_files (unistd.h HAVE_UNISTD_H)
check_include_files (sys/param.h HAVE_SYS_PARAM_H)
check_include_files (sys/stat.h HAVE_SYS_STAT_H)
check_include_files (sys/time.h HAVE_SYS_TIME_H)
check_include_files (sys/time.h HAVE_SYS_TYPES_H)

check_function_exists (close HAVE_CLOSE)
check_function_exists (getpid HAVE_GETPID)
check_function_exists (gettimeofday HAVE_GETTIMEOFDAY)
check_function_exists (open HAVE_OPEN)
check_function_exists (read HAVE_READ)
check_function_exists (sched_yield HAVE_SCHED_YIELD)

# Check for the int-type includes
check_include_files (sys/types.h HAVE_SYS_TYPES_H)
check_include_files (inttypes.h HAVE_INTTYPES_H)
check_include_files (stdint.h HAVE_STDINT_H)


# Check our 64 bit integer sizes
check_type_size (__int64 __INT64)
check_type_size (int64_t INT64_T)
Expand All @@ -124,17 +137,32 @@ check_type_size (int32_t INT32_T)
check_type_size (__int32 __INT32)
check_type_size ("long" LONG_INT)
check_type_size ("int" INT)

if (HAVE_INT32_T)
set (JSON_INT32 int32_t)
elseif (HAVE___INT32)
set (JSON_INT32 __int32)
elseif (HAVE_LONG AND (${LONG_INT} EQUAL 4))
elseif (HAVE_LONG_INT AND (${LONG_INT} EQUAL 4))
set (JSON_INT32 long)
elseif (HAVE_INT AND (${INT} EQUAL 4))
set (JSON_INT32 int)
else ()
message (FATAL_ERROR "Could not detect a valid 32 bit integer type")
message (FATAL_ERROR "Could not detect a valid 32-bit integer type")
endif ()

check_type_size (uint32_t UINT32_T)
check_type_size (__uint32 __UINT32)
check_type_size ("unsigned long" UNSIGNED_LONG_INT)
check_type_size ("unsigned int" UNSIGNED_INT)
if (HAVE_UINT32_T)
set (JSON_UINT32 uint32_t)
elseif (HAVE___UINT32)
set (JSON_UINT32 __uint32)
elseif (HAVE_UNSIGNED_LONG_INT AND (${UNSIGNED_LONG_INT} EQUAL 4))
set (JSON_UINT32 "unsigned long")
elseif (HAVE_UNSIGNED_INT AND (${UNSIGNED_INT} EQUAL 4))
set (JSON_UINT32 "unsigned int")
else ()
message (FATAL_ERROR "Could not detect a valid unsigned 32-bit integer type")
endif ()

# Check for ssize_t and SSIZE_T existance.
Expand Down Expand Up @@ -206,11 +234,9 @@ else ()
set (JSON_HAVE_LOCALECONV 0)
endif ()


# check if we have setlocale
check_function_exists (setlocale HAVE_SETLOCALE)


# Check what the inline keyword is.
# Note that the original JSON_INLINE was always set to just 'inline', so this goes further.
check_function_keywords("inline")
Expand Down Expand Up @@ -238,6 +264,9 @@ elseif (HAVE__SNPRINTF)
set (JSON_SNPRINTF _snprintf)
endif ()

check_c_source_compiles ("int main() { unsigned long val; __sync_bool_compare_and_swap(&val, 0, 1); return 0; } " HAVE_SYNC_BUILTINS)
check_c_source_compiles ("int main() { char l; unsigned long v; __atomic_test_and_set(&l, __ATOMIC_RELAXED); __atomic_store_n(&v, 1, __ATOMIC_ACQ_REL); __atomic_load_n(&v, __ATOMIC_ACQUIRE); return 0; }" HAVE_ATOMIC_BUILTINS)

# Create pkg-conf file.
# (We use the same files as ./configure does, so we
# have to defined the same variables used there).
Expand Down
46 changes: 25 additions & 21 deletions cmake/config.h.cmake
@@ -1,35 +1,36 @@
/* Reduced down to the defines that are actually used in the code */

/* Define to 1 if you have the <inttypes.h> (and friends) header file. */
#cmakedefine HAVE_INTTYPES_H 1
#cmakedefine HAVE_STDINT_H 1
#cmakedefine HAVE_ENDIAN_H 1
#cmakedefine HAVE_FCNTL_H 1
#cmakedefine HAVE_SCHED_H 1
#cmakedefine HAVE_UNISTD_H 1
#cmakedefine HAVE_SYS_PARAM_H 1
#cmakedefine HAVE_SYS_STAT_H 1
#cmakedefine HAVE_SYS_TIME_H 1
#cmakedefine HAVE_SYS_TYPES_H 1
#cmakedefine HAVE_STDINT_H 1

/* We must include this here, as in (eg) utf.h it will want to use
the integer type, which in MSVC2010 will be in stdint.h
(there is no inttypes.h in MSVC2010) */
#if defined(HAVE_STDINT_H)
# include <stdint.h>
#elif defined(HAVE_INTTYPES_H)
# include <inttypes.h>
#elif defined(HAVE_SYS_TYPES_H)
# include <sys/types.h>
#endif
#cmakedefine HAVE_CLOSE 1
#cmakedefine HAVE_GETPID 1
#cmakedefine HAVE_GETTIMEOFDAY 1
#cmakedefine HAVE_OPEN 1
#cmakedefine HAVE_READ 1
#cmakedefine HAVE_SCHED_YIELD 1

/* Define to 1 if you have the <locale.h> header file. */
#cmakedefine HAVE_LOCALE_H 1
#cmakedefine HAVE_SYNC_BUILTINS 1
#cmakedefine HAVE_ATOMIC_BUILTINS 1

/* Define to 1 if you have the 'setlocale' function. */
#cmakedefine HAVE_LOCALE_H 1
#cmakedefine HAVE_SETLOCALE 1

/* Define to the type of a signed integer type of width exactly 32 bits if
such a type exists and the standard includes do not define it. */
#cmakedefine HAVE_INT32_T 1

#ifndef HAVE_INT32_T
# define int32_t @JSON_INT32@
#endif

#cmakedefine HAVE_UINT32_T 1
#ifndef HAVE_UINT32_T
# define uint32_t @JSON_UINT32@
#endif

#cmakedefine HAVE_SSIZE_T 1

#ifndef HAVE_SSIZE_T
Expand All @@ -43,3 +44,6 @@
#endif

#cmakedefine HAVE_VSNPRINTF

#cmakedefine USE_URANDOM 1
#cmakedefine USE_WINDOWS_CRYPTOAPI 1
50 changes: 48 additions & 2 deletions configure.ac
Expand Up @@ -14,10 +14,11 @@ AM_CONDITIONAL([GCC], [test x$GCC = xyes])
# Checks for libraries.

# Checks for header files.
AC_CHECK_HEADERS([locale.h])
AC_CHECK_HEADERS([endian.h fcntl.h locale.h sched.h unistd.h sys/param.h sys/stat.h sys/time.h sys/types.h])

# Checks for typedefs, structures, and compiler characteristics.
AC_TYPE_INT32_T
AC_TYPE_UINT32_T
AC_TYPE_LONG_LONG_INT

AC_C_INLINE
Expand All @@ -29,7 +30,31 @@ esac
AC_SUBST([json_inline])

# Checks for library functions.
AC_CHECK_FUNCS([strtoll localeconv])
AC_CHECK_FUNCS([close getpid gettimeofday localeconv open read sched_yield strtoll])

AC_MSG_CHECKING([for gcc __sync builtins])
have_sync_builtins=no
AC_TRY_LINK(
[], [unsigned long val; __sync_bool_compare_and_swap(&val, 0, 1);],
[have_sync_builtins=yes],
)
if test "x$have_sync_builtins" = "xyes"; then
AC_DEFINE([HAVE_SYNC_BUILTINS], [1],
[Define to 1 if gcc's __sync builtins are available])
fi
AC_MSG_RESULT([$have_sync_builtins])

AC_MSG_CHECKING([for gcc __atomic builtins])
have_atomic_builtins=no
AC_TRY_LINK(
[], [char l; unsigned long v; __atomic_test_and_set(&l, __ATOMIC_RELAXED); __atomic_store_n(&v, 1, __ATOMIC_ACQ_REL); __atomic_load_n(&v, __ATOMIC_ACQUIRE);],
[have_atomic_builtins=yes],
)
if test "x$have_atomic_builtins" = "xyes"; then
AC_DEFINE([HAVE_ATOMIC_BUILTINS], [1],
[Define to 1 if gcc's __atomic builtins are available])
fi
AC_MSG_RESULT([$have_atomic_builtins])

case "$ac_cv_type_long_long_int$ac_cv_func_strtoll" in
yesyes) json_have_long_long=1;;
Expand All @@ -43,6 +68,27 @@ case "$ac_cv_header_locale_h$ac_cv_func_localeconv" in
esac
AC_SUBST([json_have_localeconv])

# Features
AC_ARG_ENABLE([urandom],
[AS_HELP_STRING([--disable-urandom],
[Don't use /dev/urandom to seed the hash function])],
[use_urandom=$enableval], [use_urandom=yes])

if test "x$use_urandom" = xyes; then
AC_DEFINE([USE_URANDOM], [1],
[Define to 1 if /dev/urandom should be used for seeding the hash function])
fi

AC_ARG_ENABLE([windows-cryptoapi],
[AS_HELP_STRING([--disable-windows-cryptoapi],
[Don't use CryptGenRandom to seed the hash function])],
[use_windows_cryptoapi=$enableval], [use_windows_cryptoapi=yes])

if test "x$use_windows_cryptoapi" = xyes; then
AC_DEFINE([USE_WINDOWS_CRYPTOAPI], [1],
[Define to 1 if CryptGenRandom should be used for seeding the hash function])
fi

AC_CONFIG_FILES([
jansson.pc
Makefile
Expand Down
1 change: 1 addition & 0 deletions src/Makefile.am
Expand Up @@ -8,6 +8,7 @@ libjansson_la_SOURCES = \
error.c \
hashtable.c \
hashtable.h \
hashtable_seed.c \
jansson_private.h \
load.c \
memory.c \
Expand Down

2 comments on commit 8f80c2d

@jordimassaguerpla
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very long ... is this all needed for fixing CVE-2013-6401?

@akheron
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to hashtable.c, hashtable_seed.c, lookup3.h and value.c are most important. The rest is mostly feature detection for both build systems (autoconf, cmake) and unit test fixes.

Please sign in to comment.