Skip to content

Commit

Permalink
audit memcmp usage (aws#3059)
Browse files Browse the repository at this point in the history
  • Loading branch information
toidiu authored Sep 22, 2021
1 parent 279a749 commit 01da77d
Showing 1 changed file with 49 additions and 0 deletions.
49 changes: 49 additions & 0 deletions codebuild/bin/grep_simple_mistakes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,49 @@

FAILED=0

#############################################
# Grep for any instances of raw memcmp() function. s2n code should instead be
# using s2n_constant_time_equals()
#
# KNOWN_MEMCMP_USAGE is used to capture all known uses of memcmp and acts as a
# safeguard against any new uses of memcmp.
#############################################
S2N_FILES_ASSERT_NOT_USING_MEMCMP=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/bindings/*")
declare -A KNOWN_MEMCMP_USAGE
KNOWN_MEMCMP_USAGE["$PWD/crypto/s2n_rsa.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_early_data.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_kem.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_cipher_suites.c"]=3
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_server_hello.c"]=4
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_security_policies.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_psk.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_config.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_resume.c"]=2
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_connection.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_protocol_preferences.c"]=1
KNOWN_MEMCMP_USAGE["$PWD/utils/s2n_map.c"]=3
KNOWN_MEMCMP_USAGE["$PWD/stuffer/s2n_stuffer_text.c"]=1

for file in $S2N_FILES_ASSERT_NOT_USING_MEMCMP; do
# NOTE: this matches on 'memcmp', which will also match comments. However, there
# are no uses of 'memcmp' in comments so we opt for this stricter check.
RESULT_NUM_LINES=`grep -n 'memcmp' $file | wc -l`

# set default KNOWN_MEMCMP_USAGE value
[ -z "${KNOWN_MEMCMP_USAGE["$file"]}" ] && KNOWN_MEMCMP_USAGE["$file"]="0"

# check if memcmp usage is 0 or a known value
if [ "${RESULT_NUM_LINES}" != "${KNOWN_MEMCMP_USAGE["$file"]}" ]; then
echo "Expected: ${KNOWN_MEMCMP_USAGE["$file"]} Found: ${RESULT_NUM_LINES} usage of 'memcmp' in $file"
FAILED=1
fi
done

#############################################
# Assert that functions do not return -1 or S2N_ERR* codes directly.
# To indicate failure, functions should use the S2N_ERROR* macros defined
# in s2n_errno.h.
#############################################
S2N_FILES_ASSERT_RETURN=$(find "$PWD" -type f -name "s2n*.c" -not -path "*/tests/*")
for file in $S2N_FILES_ASSERT_RETURN; do
RESULT_NEGATIVE_ONE=`grep -rn 'return -1;' $file`
Expand All @@ -37,7 +77,9 @@ for file in $S2N_FILES_ASSERT_RETURN; do
fi
done

#############################################
# Detect any array size calculations that are not using the s2n_array_len() function.
#############################################
S2N_FILES_ARRAY_SIZING_RETURN=$(find "$PWD" -type f -name "s2n*.c" -path "*")
for file in $S2N_FILES_ARRAY_SIZING_RETURN; do
RESULT_ARR_DIV=`grep -Ern 'sizeof\((.*)\) \/ sizeof\(\1\[0\]\)' $file`
Expand All @@ -48,10 +90,12 @@ for file in $S2N_FILES_ARRAY_SIZING_RETURN; do
fi
done

#############################################
# Assert that all assignments from s2n_stuffer_raw_read() have a
# notnull_check (or similar manual null check) on the same, or next, line.
# The assertion is shallow; this doesn't guarantee that we're doing the
# *correct* null check, just that we are doing *some* null check.
#############################################
S2N_FILES_ASSERT_NOTNULL_CHECK=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*")
for file in $S2N_FILES_ASSERT_NOTNULL_CHECK; do
while read -r line_one; do
Expand All @@ -77,8 +121,10 @@ for file in $S2N_FILES_ASSERT_NOTNULL_CHECK; do
done < <(grep -rnE -A 1 "=\ss2n_stuffer_raw_read\(.*\)" $file)
done

#############################################
# Assert that "index" is not a variable name. An "index" function exists in strings.h, and older compilers (<GCC 4.8)
# warn if any local variables called "index" are used because they are considered to shadow that declaration.
#############################################
S2N_FILES_ASSERT_VARIABLE_NAME_INDEX=$(find "$PWD" -type f -name "s2n*.[ch]")
for file in $S2N_FILES_ASSERT_VARIABLE_NAME_INDEX; do
RESULT_VARIABLE_NAME_INDEX=`gcc -fpreprocessed -dD -E -w $file | grep -v '"' | grep '[\*|,|;|[:space:]]index[;|,|\)|[:space:]]'`
Expand All @@ -88,8 +134,10 @@ for file in $S2N_FILES_ASSERT_VARIABLE_NAME_INDEX; do
fi
done

#############################################
## Assert that there are no new uses of S2N_ERROR_IF
# TODO add crypto, tls (see https://github.com/aws/s2n-tls/issues/2635)
#############################################
S2N_ERROR_IF_FREE="bin error pq-crypto scram stuffer utils tests"
for dir in $S2N_ERROR_IF_FREE; do
files=$(find "$dir" -type f -name "*.c" -path "*")
Expand All @@ -102,6 +150,7 @@ for dir in $S2N_ERROR_IF_FREE; do
done
done

#############################################
if [ $FAILED == 1 ]; then
printf "\\033[31;1mFAILED Grep For Simple Mistakes check\\033[0m\\n"
exit -1
Expand Down

0 comments on commit 01da77d

Please sign in to comment.