Skip to content

Commit ec1c1e5

Browse files
dgarskedgarske
authored
Merge pull request wolfSSL#212 from miyazakh/f-569_stackbuffer_overflow
F 569 : Fix stack buffer overflow in encryption setup
2 parents a176670 + 6f2525c commit ec1c1e5

3 files changed

Lines changed: 191 additions & 23 deletions

File tree

‎src/crypto/clu_crypto_setup.c‎

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,35 @@
2525

2626
#ifndef WOLFCLU_NO_FILESYSTEM
2727

28+
/* Prompt for a filename on stdin with validation.
29+
* Returns WOLFCLU_SUCCESS on success, WOLFCLU_FATAL_ERROR on EOF/read error.
30+
* buf is filled with the stripped, non-empty filename on success. */
31+
static int wolfCLU_readFilename(char* buf, int bufSz, const char* prompt)
32+
{
33+
while (1) {
34+
WOLFCLU_LOG(WOLFCLU_L0, "%s", prompt);
35+
if (fgets(buf, bufSz, stdin) == NULL) {
36+
wolfCLU_LogError("failed to read file name");
37+
return WOLFCLU_FATAL_ERROR;
38+
}
39+
/* If no newline, line was too long: flush remainder and re-prompt */
40+
if (strchr(buf, '\n') == NULL) {
41+
int ch;
42+
do {
43+
ch = getchar();
44+
} while (ch != '\n' && ch != EOF);
45+
wolfCLU_LogError("input too long, please try again");
46+
continue;
47+
}
48+
buf[strcspn(buf, "\r\n")] = '\0';
49+
if (buf[0] == '\0') {
50+
wolfCLU_LogError("empty input, please enter a file name");
51+
continue;
52+
}
53+
return WOLFCLU_SUCCESS;
54+
}
55+
}
56+
2857
static const struct option crypt_options[] = {
2958
{"-sha", no_argument, 0, WOLFCLU_CERT_SHA },
3059
{"-sha224", no_argument, 0, WOLFCLU_CERT_SHA224},
@@ -342,14 +371,15 @@ int wolfCLU_setup(int argc, char** argv, char action)
342371
}
343372

344373
if (inCheck == 0 && encCheck == 1) {
345-
ret = 0;
346-
while (ret == 0) {
347-
WOLFCLU_LOG(WOLFCLU_L0,
348-
"-in flag was not set, please enter a string or"
349-
"file name to be encrypted: ");
350-
ret = (int) scanf("%s", inName);
374+
ret = wolfCLU_readFilename(inName, sizeof(inName),
375+
"-in flag was not set, please enter a string or"
376+
" file name to be encrypted: ");
377+
if (ret != WOLFCLU_SUCCESS) {
378+
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
379+
if (mode != NULL)
380+
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
381+
return WOLFCLU_FATAL_ERROR;
351382
}
352-
in = inName;
353383
WOLFCLU_LOG(WOLFCLU_L0, "Encrypting :\"%s\"", inName);
354384
inCheck = 1;
355385
}
@@ -393,13 +423,15 @@ int wolfCLU_setup(int argc, char** argv, char action)
393423
}
394424
else {
395425
if (outCheck == 0) {
396-
ret = 0;
397-
while (ret == 0) {
398-
WOLFCLU_LOG(WOLFCLU_L0,
399-
"Please enter a name for the output file: ");
400-
ret = (int) scanf("%s", outNameEnc);
401-
out = (ret > 0) ? outNameEnc : '\0';
426+
ret = wolfCLU_readFilename(outNameEnc, sizeof(outNameEnc),
427+
"Please enter a name for the output file: ");
428+
if (ret != WOLFCLU_SUCCESS) {
429+
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
430+
if (mode != NULL)
431+
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
432+
return WOLFCLU_FATAL_ERROR;
402433
}
434+
out = outNameEnc;
403435
}
404436
ret = wolfCLU_encrypt(alg, mode, pwdKey, key, keySize, in, out,
405437
iv, block, ivCheck, inputHex);
@@ -415,13 +447,15 @@ int wolfCLU_setup(int argc, char** argv, char action)
415447
}
416448
else {
417449
if (outCheck == 0) {
418-
ret = 0;
419-
while (ret == 0) {
420-
WOLFCLU_LOG(WOLFCLU_L0,
421-
"Please enter a name for the output file: ");
422-
ret = (int) scanf("%s", outNameDec);
423-
out = (ret > 0) ? outNameDec : '\0';
450+
ret = wolfCLU_readFilename(outNameDec, sizeof(outNameDec),
451+
"Please enter a name for the output file: ");
452+
if (ret != WOLFCLU_SUCCESS) {
453+
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
454+
if (mode != NULL)
455+
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
456+
return WOLFCLU_FATAL_ERROR;
424457
}
458+
out = outNameDec;
425459
}
426460
ret = wolfCLU_decrypt(alg, mode, pwdKey, key, keySize, in, out,
427461
iv, block, keyType);

‎src/tools/clu_funcs.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,14 +1249,16 @@ void wolfCLU_AddNameEntry(WOLFSSL_X509_NAME* name, int type, int nid, char* str)
12491249
WOLFSSL_X509_NAME_ENTRY *entry;
12501250

12511251
if (str != NULL) {
1252-
/* strip off newline character if found at the end of str */
1253-
i = (int)XSTRLEN((const char*)str);
1252+
/* strip off newline/carriage-return characters at the end of str */
1253+
i = (int)XSTRLEN((const char*)str) - 1;
12541254
while (i >= 0) {
1255-
if (str[i] == '\n') {
1255+
if (str[i] == '\n' || str[i] == '\r') {
12561256
str[i] = '\0';
1257+
i--;
1258+
}
1259+
else {
12571260
break;
12581261
}
1259-
i--;
12601262
}
12611263

12621264
/* treats a '.' string as 'do not add' */

‎tests/encrypt/enc-test.sh‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,5 +209,137 @@ if grep -q "HAVE_CAMELLIA" wolfssl/wolfssl/options.h 2>/dev/null; then
209209
rm -f test_maxlen_camellia.bin test_maxlen_camellia.enc test_maxlen_camellia.dec
210210
fi
211211

212+
# Regression tests for stack buffer overflow fix (scanf -> fgets)
213+
214+
# Test: -in not provided, filename supplied via stdin to exercise the inName Path
215+
rm -f test-stdin-in.enc test-stdin-in.dec
216+
printf "certs/crl.der\n" | ./wolfssl enc -aes-128-cbc -out test-stdin-in.enc -k "testpass" > /dev/null 2>&1
217+
if [ $? != 0 ]; then
218+
echo "Failed: enc with stdin input (no -in flag)"
219+
exit 99
220+
fi
221+
rm -f test-stdin-in.dec
222+
./wolfssl enc -d -aes-128-cbc -in test-stdin-in.enc -out test-stdin-in.dec -k "testpass" > /dev/null 2>&1
223+
diff certs/crl.der test-stdin-in.dec > /dev/null 2>&1
224+
if [ $? != 0 ]; then
225+
echo "Failed: stdin enc/dec roundtrip mismatch"
226+
exit 99
227+
fi
228+
rm -f test-stdin-in.enc test-stdin-in.dec
229+
230+
231+
# Test: outNameEnc/outNameDec via stdin (non-EVP path, Camellia)
232+
./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe.enc -k "testpass" > /dev/null 2>&1
233+
if [ $? -eq 0 ]; then
234+
# outNameEnc: -out omitted, filename supplied via stdin
235+
printf "test-cam-stdin.enc\n" | ./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1
236+
if [ $? != 0 ]; then
237+
echo "Failed: Camellia enc with stdin output name (no -out flag)"
238+
exit 99
239+
fi
240+
241+
# outNameDec: -out omitted, filename supplied via stdin
242+
printf "test-cam-stdin.dec\n" | ./wolfssl enc -d -camellia-128-cbc -in test-cam-stdin.enc -k "testpass" > /dev/null 2>&1
243+
if [ $? != 0 ]; then
244+
echo "Failed: Camellia dec with stdin output name (no -out flag)"
245+
exit 99
246+
fi
247+
diff certs/crl.der test-cam-stdin.dec > /dev/null 2>&1
248+
if [ $? != 0 ]; then
249+
echo "Failed: Camellia stdin outName enc/dec roundtrip mismatch"
250+
exit 99
251+
fi
252+
253+
rm -f test-cam-stdin.enc test-cam-stdin.dec
254+
fi
255+
256+
rm -f test-cam-probe.enc
257+
258+
# Test: inName empty line is rejected, re-prompt accepts valid filename
259+
printf "\ncerts/crl.der\n" | ./wolfssl enc -aes-128-cbc -out test-empty-in.enc -k "testpass" > /dev/null 2>&1
260+
if [ $? != 0 ]; then
261+
echo "Failed: enc should accept filename after empty line on stdin (-in path)"
262+
exit 99
263+
fi
264+
./wolfssl enc -d -aes-128-cbc -in test-empty-in.enc -out test-empty-in.dec -k "testpass" > /dev/null 2>&1
265+
diff certs/crl.der test-empty-in.dec > /dev/null 2>&1
266+
if [ $? != 0 ]; then
267+
echo "Failed: enc/dec roundtrip mismatch after empty-line re-prompt (-in path)"
268+
exit 99
269+
fi
270+
rm -f test-empty-in.enc test-empty-in.dec
271+
272+
# Test: outNameEnc/outNameDec empty line is rejected (non-EVP path, Camellia)
273+
./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe2.enc -k "testpass" > /dev/null 2>&1
274+
if [ $? -eq 0 ]; then
275+
rm -f test-cam-probe2.enc
276+
277+
# outNameEnc: empty line rejected, then valid output name accepted
278+
printf "\ntest-cam-empty.enc\n" | ./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1
279+
if [ $? != 0 ]; then
280+
echo "Failed: Camellia enc should accept output name after empty line (outNameEnc)"
281+
exit 99
282+
fi
283+
284+
# outNameDec: empty line rejected, then valid output name accepted
285+
printf "\ntest-cam-empty.dec\n" | ./wolfssl enc -d -camellia-128-cbc -in test-cam-empty.enc -k "testpass" > /dev/null 2>&1
286+
if [ $? != 0 ]; then
287+
echo "Failed: Camellia dec should accept output name after empty line (outNameDec)"
288+
exit 99
289+
fi
290+
diff certs/crl.der test-cam-empty.dec > /dev/null 2>&1
291+
if [ $? != 0 ]; then
292+
echo "Failed: enc/dec roundtrip mismatch after empty-line re-prompt (outNameEnc/Dec)"
293+
exit 99
294+
fi
295+
rm -f test-cam-empty.enc test-cam-empty.dec
296+
fi
297+
298+
# Test: 'input too long' path — inName buffer overflow prevention
299+
# Pipe a 255-char line (no newline within fgets buffer), triggering the
300+
# strchr(buf,'\n')==NULL flush branch, then supply a valid filename.
301+
LONG_INPUT=$(printf '%255s' ' ')
302+
printf "%s\ncerts/crl.der\n" "$LONG_INPUT" | \
303+
./wolfssl enc -aes-128-cbc -out test-toolong-in.enc -k "testpass" > /dev/null 2>&1
304+
if [ $? != 0 ]; then
305+
echo "Failed: enc should recover and accept filename after too-long input (-in path)"
306+
exit 99
307+
fi
308+
./wolfssl enc -d -aes-128-cbc -in test-toolong-in.enc -out test-toolong-in.dec -k "testpass" > /dev/null 2>&1
309+
diff certs/crl.der test-toolong-in.dec > /dev/null 2>&1
310+
if [ $? != 0 ]; then
311+
echo "Failed: enc/dec roundtrip mismatch after too-long re-prompt (-in path)"
312+
exit 99
313+
fi
314+
rm -f test-toolong-in.enc test-toolong-in.dec
315+
316+
# Test: 'input too long' path — outNameEnc/outNameDec (non-EVP path, Camellia)
317+
./wolfssl enc -camellia-128-cbc -in certs/crl.der -out test-cam-probe3.enc -k "testpass" > /dev/null 2>&1
318+
if [ $? -eq 0 ]; then
319+
rm -f test-cam-probe3.enc
320+
321+
# outNameEnc: too-long input flushed, then valid output name accepted
322+
printf "%s\ntest-cam-toolong.enc\n" "$LONG_INPUT" | \
323+
./wolfssl enc -camellia-128-cbc -in certs/crl.der -k "testpass" > /dev/null 2>&1
324+
if [ $? != 0 ]; then
325+
echo "Failed: Camellia enc should recover after too-long output name (outNameEnc)"
326+
exit 99
327+
fi
328+
329+
# outNameDec: too-long input flushed, then valid output name accepted
330+
printf "%s\ntest-cam-toolong.dec\n" "$LONG_INPUT" | \
331+
./wolfssl enc -d -camellia-128-cbc -in test-cam-toolong.enc -k "testpass" > /dev/null 2>&1
332+
if [ $? != 0 ]; then
333+
echo "Failed: Camellia dec should recover after too-long output name (outNameDec)"
334+
exit 99
335+
fi
336+
diff certs/crl.der test-cam-toolong.dec > /dev/null 2>&1
337+
if [ $? != 0 ]; then
338+
echo "Failed: enc/dec roundtrip mismatch after too-long re-prompt (outNameEnc/Dec)"
339+
exit 99
340+
fi
341+
rm -f test-cam-toolong.enc test-cam-toolong.dec
342+
fi
343+
212344
echo "Done"
213345
exit 0

0 commit comments

Comments
 (0)