diff --git a/src/crypto/clu_crypto_setup.c b/src/crypto/clu_crypto_setup.c index 95cf8ac6..cce5dc3e 100644 --- a/src/crypto/clu_crypto_setup.c +++ b/src/crypto/clu_crypto_setup.c @@ -25,6 +25,35 @@ #ifndef WOLFCLU_NO_FILESYSTEM +/* Prompt for a filename on stdin with validation. + * Returns WOLFCLU_SUCCESS on success, WOLFCLU_FATAL_ERROR on EOF/read error. + * buf is filled with the stripped, non-empty filename on success. */ +static int wolfCLU_readFilename(char* buf, int bufSz, const char* prompt) +{ + while (1) { + WOLFCLU_LOG(WOLFCLU_L0, "%s", prompt); + if (fgets(buf, bufSz, stdin) == NULL) { + wolfCLU_LogError("failed to read file name"); + return WOLFCLU_FATAL_ERROR; + } + /* If no newline, line was too long: flush remainder and re-prompt */ + if (strchr(buf, '\n') == NULL) { + int ch; + do { + ch = getchar(); + } while (ch != '\n' && ch != EOF); + wolfCLU_LogError("input too long, please try again"); + continue; + } + buf[strcspn(buf, "\r\n")] = '\0'; + if (buf[0] == '\0') { + wolfCLU_LogError("empty input, please enter a file name"); + continue; + } + return WOLFCLU_SUCCESS; + } +} + static const struct option crypt_options[] = { {"-sha", no_argument, 0, WOLFCLU_CERT_SHA }, {"-sha224", no_argument, 0, WOLFCLU_CERT_SHA224}, @@ -342,14 +371,15 @@ int wolfCLU_setup(int argc, char** argv, char action) } if (inCheck == 0 && encCheck == 1) { - ret = 0; - while (ret == 0) { - WOLFCLU_LOG(WOLFCLU_L0, - "-in flag was not set, please enter a string or" - "file name to be encrypted: "); - ret = (int) scanf("%s", inName); + ret = wolfCLU_readFilename(inName, sizeof(inName), + "-in flag was not set, please enter a string or" + " file name to be encrypted: "); + if (ret != WOLFCLU_SUCCESS) { + wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL); + if (mode != NULL) + XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return WOLFCLU_FATAL_ERROR; } - in = inName; WOLFCLU_LOG(WOLFCLU_L0, "Encrypting :\"%s\"", inName); inCheck = 1; } @@ -393,13 +423,15 @@ int wolfCLU_setup(int argc, char** argv, char action) } else { if (outCheck == 0) { - ret = 0; - while (ret == 0) { - WOLFCLU_LOG(WOLFCLU_L0, - "Please enter a name for the output file: "); - ret = (int) scanf("%s", outNameEnc); - out = (ret > 0) ? outNameEnc : '\0'; + ret = wolfCLU_readFilename(outNameEnc, sizeof(outNameEnc), + "Please enter a name for the output file: "); + if (ret != WOLFCLU_SUCCESS) { + wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL); + if (mode != NULL) + XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return WOLFCLU_FATAL_ERROR; } + out = outNameEnc; } ret = wolfCLU_encrypt(alg, mode, pwdKey, key, keySize, in, out, iv, block, ivCheck, inputHex); @@ -415,13 +447,15 @@ int wolfCLU_setup(int argc, char** argv, char action) } else { if (outCheck == 0) { - ret = 0; - while (ret == 0) { - WOLFCLU_LOG(WOLFCLU_L0, - "Please enter a name for the output file: "); - ret = (int) scanf("%s", outNameDec); - out = (ret > 0) ? outNameDec : '\0'; + ret = wolfCLU_readFilename(outNameDec, sizeof(outNameDec), + "Please enter a name for the output file: "); + if (ret != WOLFCLU_SUCCESS) { + wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL); + if (mode != NULL) + XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return WOLFCLU_FATAL_ERROR; } + out = outNameDec; } ret = wolfCLU_decrypt(alg, mode, pwdKey, key, keySize, in, out, iv, block, keyType); diff --git a/src/tools/clu_funcs.c b/src/tools/clu_funcs.c index f52d467e..10d9b691 100644 --- a/src/tools/clu_funcs.c +++ b/src/tools/clu_funcs.c @@ -1249,14 +1249,16 @@ void wolfCLU_AddNameEntry(WOLFSSL_X509_NAME* name, int type, int nid, char* str) WOLFSSL_X509_NAME_ENTRY *entry; if (str != NULL) { - /* strip off newline character if found at the end of str */ - i = (int)XSTRLEN((const char*)str); + /* strip off newline/carriage-return characters at the end of str */ + i = (int)XSTRLEN((const char*)str) - 1; while (i >= 0) { - if (str[i] == '\n') { + if (str[i] == '\n' || str[i] == '\r') { str[i] = '\0'; + i--; + } + else { break; } - i--; } /* treats a '.' string as 'do not add' */ diff --git a/tests/encrypt/enc-test.sh b/tests/encrypt/enc-test.sh index 13631cf0..234b6e02 100755 --- a/tests/encrypt/enc-test.sh +++ b/tests/encrypt/enc-test.sh @@ -209,5 +209,137 @@ if grep -q "HAVE_CAMELLIA" wolfssl/wolfssl/options.h 2>/dev/null; then rm -f test_maxlen_camellia.bin test_maxlen_camellia.enc test_maxlen_camellia.dec fi +# Regression tests for stack buffer overflow fix (scanf -> fgets) + +# Test: -in not provided, filename supplied via stdin to exercise the inName Path +rm -f test-stdin-in.enc test-stdin-in.dec +printf "certs/crl.der\n" | ./wolfssl enc -aes-128-cbc -out test-stdin-in.enc -k "testpass" > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: enc with stdin input (no -in flag)" + exit 99 +fi +rm -f test-stdin-in.dec +./wolfssl enc -d -aes-128-cbc -in test-stdin-in.enc -out test-stdin-in.dec -k "testpass" > /dev/null 2>&1 +diff certs/crl.der test-stdin-in.dec > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: stdin enc/dec roundtrip mismatch" + exit 99 +fi +rm -f test-stdin-in.enc test-stdin-in.dec + + +# Test: outNameEnc/outNameDec via stdin (non-EVP path, Camellia) +./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe.enc -k "testpass" > /dev/null 2>&1 +if [ $? -eq 0 ]; then + # outNameEnc: -out omitted, filename supplied via stdin + printf "test-cam-stdin.enc\n" | ./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia enc with stdin output name (no -out flag)" + exit 99 + fi + + # outNameDec: -out omitted, filename supplied via stdin + printf "test-cam-stdin.dec\n" | ./wolfssl enc -d -camellia-128-cbc -in test-cam-stdin.enc -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia dec with stdin output name (no -out flag)" + exit 99 + fi + diff certs/crl.der test-cam-stdin.dec > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia stdin outName enc/dec roundtrip mismatch" + exit 99 + fi + + rm -f test-cam-stdin.enc test-cam-stdin.dec +fi + +rm -f test-cam-probe.enc + +# Test: inName empty line is rejected, re-prompt accepts valid filename +printf "\ncerts/crl.der\n" | ./wolfssl enc -aes-128-cbc -out test-empty-in.enc -k "testpass" > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: enc should accept filename after empty line on stdin (-in path)" + exit 99 +fi +./wolfssl enc -d -aes-128-cbc -in test-empty-in.enc -out test-empty-in.dec -k "testpass" > /dev/null 2>&1 +diff certs/crl.der test-empty-in.dec > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: enc/dec roundtrip mismatch after empty-line re-prompt (-in path)" + exit 99 +fi +rm -f test-empty-in.enc test-empty-in.dec + +# Test: outNameEnc/outNameDec empty line is rejected (non-EVP path, Camellia) +./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe2.enc -k "testpass" > /dev/null 2>&1 +if [ $? -eq 0 ]; then + rm -f test-cam-probe2.enc + + # outNameEnc: empty line rejected, then valid output name accepted + printf "\ntest-cam-empty.enc\n" | ./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia enc should accept output name after empty line (outNameEnc)" + exit 99 + fi + + # outNameDec: empty line rejected, then valid output name accepted + printf "\ntest-cam-empty.dec\n" | ./wolfssl enc -d -camellia-128-cbc -in test-cam-empty.enc -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia dec should accept output name after empty line (outNameDec)" + exit 99 + fi + diff certs/crl.der test-cam-empty.dec > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: enc/dec roundtrip mismatch after empty-line re-prompt (outNameEnc/Dec)" + exit 99 + fi + rm -f test-cam-empty.enc test-cam-empty.dec +fi + +# Test: 'input too long' path — inName buffer overflow prevention +# Pipe a 255-char line (no newline within fgets buffer), triggering the +# strchr(buf,'\n')==NULL flush branch, then supply a valid filename. +LONG_INPUT=$(printf '%255s' ' ') +printf "%s\ncerts/crl.der\n" "$LONG_INPUT" | \ + ./wolfssl enc -aes-128-cbc -out test-toolong-in.enc -k "testpass" > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: enc should recover and accept filename after too-long input (-in path)" + exit 99 +fi +./wolfssl enc -d -aes-128-cbc -in test-toolong-in.enc -out test-toolong-in.dec -k "testpass" > /dev/null 2>&1 +diff certs/crl.der test-toolong-in.dec > /dev/null 2>&1 +if [ $? != 0 ]; then + echo "Failed: enc/dec roundtrip mismatch after too-long re-prompt (-in path)" + exit 99 +fi +rm -f test-toolong-in.enc test-toolong-in.dec + +# Test: 'input too long' path — outNameEnc/outNameDec (non-EVP path, Camellia) +./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe3.enc -k "testpass" > /dev/null 2>&1 +if [ $? -eq 0 ]; then + rm -f test-cam-probe3.enc + + # outNameEnc: too-long input flushed, then valid output name accepted + printf "%s\ntest-cam-toolong.enc\n" "$LONG_INPUT" | \ + ./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia enc should recover after too-long output name (outNameEnc)" + exit 99 + fi + + # outNameDec: too-long input flushed, then valid output name accepted + printf "%s\ntest-cam-toolong.dec\n" "$LONG_INPUT" | \ + ./wolfssl enc -d -camellia-128-cbc -in test-cam-toolong.enc -k "testpass" > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: Camellia dec should recover after too-long output name (outNameDec)" + exit 99 + fi + diff certs/crl.der test-cam-toolong.dec > /dev/null 2>&1 + if [ $? != 0 ]; then + echo "Failed: enc/dec roundtrip mismatch after too-long re-prompt (outNameEnc/Dec)" + exit 99 + fi + rm -f test-cam-toolong.enc test-cam-toolong.dec +fi + echo "Done" exit 0