Skip to content

Commit 6f2525c

Browse files
committed
Addressed review comments
1 parent c109e75 commit 6f2525c

2 files changed

Lines changed: 99 additions & 85 deletions

File tree

src/crypto/clu_crypto_setup.c

Lines changed: 53 additions & 85 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,38 +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-
if (fgets(inName, sizeof(inName), stdin) == NULL) {
351-
/* EOF or read error: cannot prompt further */
352-
wolfCLU_LogError("failed to read input file name");
353-
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
354-
if (mode != NULL)
355-
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
356-
return WOLFCLU_FATAL_ERROR;
357-
}
358-
/* If no newline is present, the line was too long: flush and
359-
* re-prompt rather than proceeding with a truncated filename. */
360-
if (strchr(inName, '\n') == NULL) {
361-
int ch;
362-
do {
363-
ch = getchar();
364-
} while (ch != '\n' && ch != EOF);
365-
wolfCLU_LogError("input too long, please try again");
366-
continue;
367-
}
368-
inName[strcspn(inName, "\r\n")] = '\0';
369-
/* Do not accept an empty string as valid input */
370-
if (inName[0] == '\0') {
371-
wolfCLU_LogError("empty input, please enter a file name");
372-
continue;
373-
}
374-
ret = 1;
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;
375382
}
376-
in = inName;
377383
WOLFCLU_LOG(WOLFCLU_L0, "Encrypting :\"%s\"", inName);
378384
inCheck = 1;
379385
}
@@ -417,34 +423,15 @@ int wolfCLU_setup(int argc, char** argv, char action)
417423
}
418424
else {
419425
if (outCheck == 0) {
420-
ret = 0;
421-
while (ret == 0) {
422-
WOLFCLU_LOG(WOLFCLU_L0,
423-
"Please enter a name for the output file: ");
424-
if (fgets(outNameEnc, sizeof(outNameEnc), stdin) == NULL) {
425-
/* EOF or read error: cannot prompt further */
426-
wolfCLU_LogError("failed to read output file name");
427-
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
428-
if (mode != NULL)
429-
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
430-
return WOLFCLU_FATAL_ERROR;
431-
}
432-
if (strchr(outNameEnc, '\n') == NULL) {
433-
int ch;
434-
do {
435-
ch = getchar();
436-
} while (ch != '\n' && ch != EOF);
437-
wolfCLU_LogError("input too long, please try again");
438-
continue;
439-
}
440-
outNameEnc[strcspn(outNameEnc, "\r\n")] = '\0';
441-
if (outNameEnc[0] == '\0') {
442-
wolfCLU_LogError("empty input, please enter a file name");
443-
continue;
444-
}
445-
out = outNameEnc;
446-
ret = 1;
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;
447433
}
434+
out = outNameEnc;
448435
}
449436
ret = wolfCLU_encrypt(alg, mode, pwdKey, key, keySize, in, out,
450437
iv, block, ivCheck, inputHex);
@@ -460,34 +447,15 @@ int wolfCLU_setup(int argc, char** argv, char action)
460447
}
461448
else {
462449
if (outCheck == 0) {
463-
ret = 0;
464-
while (ret == 0) {
465-
WOLFCLU_LOG(WOLFCLU_L0,
466-
"Please enter a name for the output file: ");
467-
if (fgets(outNameDec, sizeof(outNameDec), stdin) == NULL) {
468-
/* EOF or read error: cannot prompt further */
469-
wolfCLU_LogError("failed to read output file name");
470-
wolfCLU_freeBins(pwdKey, iv, key, NULL, NULL);
471-
if (mode != NULL)
472-
XFREE(mode, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
473-
return WOLFCLU_FATAL_ERROR;
474-
}
475-
if (strchr(outNameDec, '\n') == NULL) {
476-
int ch;
477-
do {
478-
ch = getchar();
479-
} while (ch != '\n' && ch != EOF);
480-
wolfCLU_LogError("input too long, please try again");
481-
continue;
482-
}
483-
outNameDec[strcspn(outNameDec, "\r\n")] = '\0';
484-
if (outNameDec[0] == '\0') {
485-
wolfCLU_LogError("empty input, please enter a file name");
486-
continue;
487-
}
488-
out = outNameDec;
489-
ret = 1;
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;
490457
}
458+
out = outNameDec;
491459
}
492460
ret = wolfCLU_decrypt(alg, mode, pwdKey, key, keySize, in, out,
493461
iv, block, keyType);

tests/encrypt/enc-test.sh

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,5 +295,51 @@ if [ $? -eq 0 ]; then
295295
rm -f test-cam-empty.enc test-cam-empty.dec
296296
fi
297297

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+
298344
echo "Done"
299345
exit 0

0 commit comments

Comments
 (0)