Skip to content

F 569 : Fix stack buffer overflow in encryption setup #212

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.

Sign up for GitHub

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

Open
miyazakh wants to merge 4 commits into
base: main
Choose a base branch
from
+153 −6
Open

F 569 : Fix stack buffer overflow in encryption setup #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11176f8
fix stack buffer overflow in encryption setup
miyazakh Mar 26, 2026
afa1d89
addressed copilot comments
miyazakh Mar 26, 2026
08894c7
addressed copilot comment
miyazakh Mar 27, 2026
2a009f1
addressed copilot comments
miyazakh Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 69 additions & 6 deletions src/crypto/clu_crypto_setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,31 @@ int wolfCLU_setup(int argc, char** argv, char action)
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);
" file name to be encrypted: ");
if (fgets(inName, sizeof(inName), stdin) == NULL) {
/* EOF or read error: cannot prompt further */
wolfCLU_LogError("failed to read input file name");
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
if (mode != NULL)
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
return WOLFCLU_FATAL_ERROR;
}
/* If no newline is present, the line was too long: flush and
* re-prompt rather than proceeding with a truncated filename. */
if (strchr(inName, '\n') == NULL) {
int ch;
do {
ch = getchar();
} while (ch != '\n' && ch != EOF);
wolfCLU_LogError("input too long, please try again");
continue;
}
inName[strcspn(inName, "\n")] = '\0';
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Stripping only \"\\n\" leaves a trailing \"\\r\" on CRLF inputs (common on Windows), which can make filenames fail to open. Use a delimiter set like \"\\r\\n\" here (and similarly for outNameEnc/outNameDec).

Suggested change
inName[strcspn(inName, "\n")] = '\0';
inName[strcspn(inName, "\r\n")] = '\0';

Copilot uses AI. Check for mistakes.
/* Do not accept an empty string as valid input */
if (inName[0] == '\0') {
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Empty input is silently ignored and the loop re-prompts without any feedback. Consider logging a short message (or re-printing the prompt with a clear hint) so interactive users understand why it’s asking again. Same applies to the empty-string checks for outNameEnc/outNameDec.

Suggested change
if (inName[0] == '\0') {
if (inName[0] == '\0') {
wolfCLU_LogError("empty input is not allowed, please try again");

Copilot uses AI. Check for mistakes.
continue;
}
ret = 1;
}
in = inName;
WOLFCLU_LOG(WOLFCLU_L0, "Encrypting :\"%s\"", inName);
Expand Down Expand Up @@ -397,8 +420,28 @@ int wolfCLU_setup(int argc, char** argv, char action)
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';
if (fgets(outNameEnc, sizeof(outNameEnc), stdin) == NULL) {
/* EOF or read error: cannot prompt further */
wolfCLU_LogError("failed to read output file name");
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
if (mode != NULL)
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
return WOLFCLU_FATAL_ERROR;
}
if (strchr(outNameEnc, '\n') == NULL) {
int ch;
do {
ch = getchar();
} while (ch != '\n' && ch != EOF);
wolfCLU_LogError("input too long, please try again");
continue;
}
outNameEnc[strcspn(outNameEnc, "\n")] = '\0';
if (outNameEnc[0] == '\0') {
continue;
}
out = outNameEnc;
ret = 1;
}
}
ret = wolfCLU_encrypt(alg, mode, pwdKey, key, keySize, in, out,
Expand All @@ -419,8 +462,28 @@ int wolfCLU_setup(int argc, char** argv, char action)
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';
if (fgets(outNameDec, sizeof(outNameDec), stdin) == NULL) {
/* EOF or read error: cannot prompt further */
wolfCLU_LogError("failed to read output file name");
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
if (mode != NULL)
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
return WOLFCLU_FATAL_ERROR;
}
if (strchr(outNameDec, '\n') == NULL) {
int ch;
do {
ch = getchar();
} while (ch != '\n' && ch != EOF);
wolfCLU_LogError("input too long, please try again");
continue;
}
outNameDec[strcspn(outNameDec, "\n")] = '\0';
if (outNameDec[0] == '\0') {
continue;
}
out = outNameDec;
ret = 1;
}
}
ret = wolfCLU_decrypt(alg, mode, pwdKey, key, keySize, in, out,
Expand Down
84 changes: 84 additions & 0 deletions tests/encrypt/enc-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,5 +185,89 @@ fi
rm -f test-dec.der
rm -f test-enc.der

# Regression tests for stack buffer overflow fix (scanf -> fgets)

# Test: -in not provided, filename supplied via stdin to exercise the inName Path
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.

Suggested change
# Test: -in not provided, filename supplied via stdin to exercise the inName Path
# Test: -in not provided, filename supplied via stdin to exercise the inName Path
rm -f test-stdin-in.enc test-stdin-in.dec

Copilot uses AI. Check for mistakes.
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
./wolfssl enc -d -aes-128-cbc -in test-stdin-in.enc -out test-stdin-in.dec -k "testpass" > /dev/null 2>&1
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.

Copilot uses AI. Check for mistakes.
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
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.

Copilot uses AI. Check for mistakes.


# 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

echo "Done"
exit 0
Loading