Skip to content

Commit e2bfa19

Browse files
dgarskedgarske
authored
Merge pull request #1002 from ejohnstown/tidy-tidy
tidy clang-tidy
2 parents ba09b58 + 1d86a8e commit e2bfa19

18 files changed

Lines changed: 552 additions & 359 deletions

File tree

‎apps/wolfsshd/configuration.c‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,16 @@ struct WOLFSSHD_CONFIG {
9191
byte authKeysFileSet:1; /* if not set then no explicit authorized keys */
9292
};
9393

94-
int CountWhitespace(const char* in, int inSz, byte inv);
95-
int SetFileString(char** dst, const char* src, void* heap);
94+
/* Maximum depth of nested Include directives. Bounds the recursion
95+
* through wolfSSHD_ConfigLoad -> ParseConfigLine -> HandleConfigOption
96+
* -> HandleInclude -> wolfSSHD_ConfigLoad. */
97+
#ifndef WOLFSSHD_MAX_INCLUDE_DEPTH
98+
#define WOLFSSHD_MAX_INCLUDE_DEPTH 16
99+
#endif
100+
static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth);
101+
102+
static int CountWhitespace(const char* in, int inSz, byte inv);
103+
static int SetFileString(char** dst, const char* src, void* heap);
96104

97105
/* convert a string into seconds, handles if 'm' for minutes follows the string
98106
* number, i.e. 2m
@@ -616,7 +624,8 @@ static int HandlePort(WOLFSSHD_CONFIG* conf, const char* value)
616624
return ret;
617625
}
618626

619-
static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
627+
/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
628+
static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
620629
{
621630
const char *ptr;
622631
const char *ptr2;
@@ -802,7 +811,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
802811
WSNPRINTF(filepath, PATH_MAX, "%s/%s", path,
803812
fileNames[i]);
804813
}
805-
ret = wolfSSHD_ConfigLoad(conf, filepath);
814+
ret = ConfigLoad(conf, filepath, depth);
806815
if (ret != WS_SUCCESS) {
807816
break;
808817
}
@@ -834,7 +843,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
834843
#endif
835844
}
836845
else {
837-
ret = wolfSSHD_ConfigLoad(conf, value);
846+
ret = ConfigLoad(conf, value, depth);
838847
}
839848
}
840849
return ret;
@@ -974,8 +983,9 @@ static int HandleForcedCommand(WOLFSSHD_CONFIG* conf, const char* value,
974983
}
975984

976985
/* returns WS_SUCCESS on success */
986+
/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
977987
static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
978-
const char* value, const char* full, int fullSz)
988+
const char* value, const char* full, int fullSz, int depth)
979989
{
980990
int ret = WS_BAD_ARGUMENT;
981991

@@ -1043,7 +1053,7 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
10431053
ret = WS_SUCCESS;
10441054
break;
10451055
case OPT_INCLUDE:
1046-
ret = HandleInclude(*conf, value);
1056+
ret = HandleInclude(*conf, value, depth);
10471057
break;
10481058
case OPT_CHROOT_DIR:
10491059
ret = HandleChrootDir(*conf, value);
@@ -1074,7 +1084,7 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
10741084

10751085
/* helper function to count white spaces, returns the number of white spaces on
10761086
* success */
1077-
int CountWhitespace(const char* in, int inSz, byte inv)
1087+
static int CountWhitespace(const char* in, int inSz, byte inv)
10781088
{
10791089
int i = 0;
10801090

@@ -1100,8 +1110,9 @@ int CountWhitespace(const char* in, int inSz, byte inv)
11001110
* Fails if any option is found that is unknown/unsupported
11011111
* Match command will create new configs for specific matching cases
11021112
*/
1113+
/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
11031114
WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
1104-
int lSz)
1115+
int lSz, int depth)
11051116
{
11061117
int ret = WS_BAD_ARGUMENT;
11071118
int sz = 0;
@@ -1132,7 +1143,8 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
11321143
else {
11331144
WMEMCPY(tmp, l + idx, sz);
11341145
tmp[sz] = 0;
1135-
ret = HandleConfigOption(conf, found->tag, tmp, l + idx, lSz - idx);
1146+
ret = HandleConfigOption(conf,
1147+
found->tag, tmp, l + idx, lSz - idx, depth);
11361148
}
11371149
}
11381150
else {
@@ -1153,6 +1165,13 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
11531165
* returns WS_SUCCESS on success
11541166
*/
11551167
int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
1168+
{
1169+
return ConfigLoad(conf, filename, 0);
1170+
}
1171+
1172+
1173+
/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
1174+
static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
11561175
{
11571176
WFILE *f;
11581177
WOLFSSHD_CONFIG* currentConfig;
@@ -1163,12 +1182,20 @@ int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
11631182
if (conf == NULL || filename == NULL)
11641183
return BAD_FUNC_ARG;
11651184

1185+
if (depth >= WOLFSSHD_MAX_INCLUDE_DEPTH) {
1186+
wolfSSH_Log(WS_LOG_ERROR,
1187+
"[SSHD] Include depth (%d) exceeded loading %s",
1188+
WOLFSSHD_MAX_INCLUDE_DEPTH, filename);
1189+
return WS_BAD_ARGUMENT;
1190+
}
1191+
11661192
if (WFOPEN(NULL, &f, filename, "rb") != 0) {
11671193
wolfSSH_Log(WS_LOG_ERROR, "Unable to open SSHD config file %s",
11681194
filename);
11691195
return BAD_FUNC_ARG;
11701196
}
11711197
wolfSSH_Log(WS_LOG_INFO, "[SSHD] parsing config file %s", filename);
1198+
depth++;
11721199

11731200
currentConfig = conf;
11741201
while ((current = XFGETS(buf, MAX_LINE_SIZE, f)) != NULL) {
@@ -1189,7 +1216,7 @@ int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
11891216
continue; /* commented out line */
11901217
}
11911218

1192-
ret = ParseConfigLine(&currentConfig, current, currentSz);
1219+
ret = ParseConfigLine(&currentConfig, current, currentSz, depth);
11931220
if (ret != WS_SUCCESS) {
11941221
fprintf(stderr, "Unable to parse config line : %s\n", current);
11951222
break;
@@ -1356,7 +1383,7 @@ char* wolfSSHD_ConfigGetUserCAKeysFile(const WOLFSSHD_CONFIG* conf)
13561383
return ret;
13571384
}
13581385

1359-
int SetFileString(char** dst, const char* src, void* heap)
1386+
static int SetFileString(char** dst, const char* src, void* heap)
13601387
{
13611388
int ret = WS_SUCCESS;
13621389

‎apps/wolfsshd/configuration.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf,
5959
void wolfSSHD_ConfigSavePID(const WOLFSSHD_CONFIG* conf);
6060

6161
#ifdef WOLFSSHD_UNIT_TEST
62-
int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l, int lSz);
62+
int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l, int lSz, int depth);
6363
#endif
6464

6565
#endif /* WOLFSSHD_H */

‎apps/wolfsshd/test/test_configuration.c‎

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
#endif /* __GNUC__ */
3636

3737

38-
void Log(const char *const, ...) FMTCHECK;
38+
void Log(const char *const fmt, ...) FMTCHECK;
3939
void Log(const char *const fmt, ...)
4040
{
4141
va_list vlist;
@@ -261,7 +261,7 @@ static int test_ParseConfigLine(void)
261261
Log(" Testing scenario: %s.", vectors[i].desc);
262262

263263
ret = ParseConfigLine(&conf, vectors[i].line,
264-
(int)WSTRLEN(vectors[i].line));
264+
(int)WSTRLEN(vectors[i].line), 0);
265265

266266
if ((ret == WS_SUCCESS && !vectors[i].shouldFail) ||
267267
(ret != WS_SUCCESS && vectors[i].shouldFail)) {
@@ -293,7 +293,7 @@ static int test_ConfigCopy(void)
293293
conf = head;
294294

295295
/* string fields via ParseConfigLine */
296-
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s))
296+
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
297297
if (ret == WS_SUCCESS) ret = PCL("Banner /etc/issue");
298298
if (ret == WS_SUCCESS) ret = PCL("ChrootDirectory /var/chroot");
299299
if (ret == WS_SUCCESS) ret = PCL("HostKey /etc/ssh/ssh_host_key");
@@ -437,7 +437,7 @@ static int test_GetUserConfMatchOverride(void)
437437
ret = WS_MEMORY_E;
438438
conf = head;
439439

440-
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s))
440+
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
441441
/* permissive global settings */
442442
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
443443
if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords yes");
@@ -501,6 +501,82 @@ static int test_GetUserConfMatchOverride(void)
501501
return ret;
502502
}
503503

504+
/* Bounded recursion through Include directives: a self-including config
505+
* must fail with WS_BAD_ARGUMENT once the depth limit is hit, and the
506+
* config object must remain usable so a subsequent load of a normal
507+
* config on the same WOLFSSHD_CONFIG still succeeds. */
508+
static int test_IncludeRecursionBound(void)
509+
{
510+
int ret = WS_SUCCESS;
511+
WOLFSSHD_CONFIG* conf = NULL;
512+
WFILE* f = NULL;
513+
const char* loopPath = "./include_loop.conf";
514+
const char* normalPath = "./include_normal.conf";
515+
const char* loopContents = "Include ./include_loop.conf\n";
516+
const char* normalContents = "Port 22\n";
517+
word32 sz, wr;
518+
519+
WFOPEN(NULL, &f, loopPath, "w");
520+
if (f == NULL) {
521+
Log(" Could not create %s.\n", loopPath);
522+
return WS_FATAL_ERROR;
523+
}
524+
sz = (word32)WSTRLEN(loopContents);
525+
wr = (word32)WFWRITE(NULL, loopContents, sizeof(char), sz, f);
526+
WFCLOSE(NULL, f);
527+
if (sz != wr) {
528+
WREMOVE(0, loopPath);
529+
return WS_FATAL_ERROR;
530+
}
531+
532+
WFOPEN(NULL, &f, normalPath, "w");
533+
if (f == NULL) {
534+
WREMOVE(0, loopPath);
535+
Log(" Could not create %s.\n", normalPath);
536+
return WS_FATAL_ERROR;
537+
}
538+
sz = (word32)WSTRLEN(normalContents);
539+
wr = (word32)WFWRITE(NULL, normalContents, sizeof(char), sz, f);
540+
WFCLOSE(NULL, f);
541+
if (sz != wr) {
542+
WREMOVE(0, loopPath);
543+
WREMOVE(0, normalPath);
544+
return WS_FATAL_ERROR;
545+
}
546+
547+
conf = wolfSSHD_ConfigNew(NULL);
548+
if (conf == NULL) {
549+
ret = WS_MEMORY_E;
550+
}
551+
552+
if (ret == WS_SUCCESS) {
553+
Log(" Testing scenario: self-include hits depth bound.");
554+
if (wolfSSHD_ConfigLoad(conf, loopPath) == WS_BAD_ARGUMENT) {
555+
Log(" PASSED.\n");
556+
}
557+
else {
558+
Log(" FAILED.\n");
559+
ret = WS_FATAL_ERROR;
560+
}
561+
}
562+
563+
if (ret == WS_SUCCESS) {
564+
Log(" Testing scenario: config reusable after failed include.");
565+
if (wolfSSHD_ConfigLoad(conf, normalPath) == WS_SUCCESS) {
566+
Log(" PASSED.\n");
567+
}
568+
else {
569+
Log(" FAILED.\n");
570+
ret = WS_FATAL_ERROR;
571+
}
572+
}
573+
574+
wolfSSHD_ConfigFree(conf);
575+
WREMOVE(0, loopPath);
576+
WREMOVE(0, normalPath);
577+
return ret;
578+
}
579+
504580
/* Verifies ConfigFree releases all string fields - most useful under ASan. */
505581
static int test_ConfigFree(void)
506582
{
@@ -513,7 +589,7 @@ static int test_ConfigFree(void)
513589
ret = WS_MEMORY_E;
514590
conf = head;
515591

516-
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s))
592+
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
517593
if (ret == WS_SUCCESS) ret = PCL("Banner /etc/issue");
518594
if (ret == WS_SUCCESS) ret = PCL("ChrootDirectory /var/chroot");
519595
if (ret == WS_SUCCESS) ret = PCL("HostKey /etc/ssh/ssh_host_key");
@@ -861,6 +937,7 @@ const TEST_CASE testCases[] = {
861937
TEST_DECL(test_ConfigCopy),
862938
TEST_DECL(test_GetUserConfMatchOverride),
863939
TEST_DECL(test_CAKeysFileDiffers),
940+
TEST_DECL(test_IncludeRecursionBound),
864941
TEST_DECL(test_ConfigFree),
865942
#ifdef WOLFSSL_BASE64_ENCODE
866943
TEST_DECL(test_CheckAuthKeysLine),

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,6 @@ static void ShowUsage(void)
203203
static void interruptCatch(int in)
204204
{
205205
(void)in;
206-
if (logFile)
207-
fprintf(logFile, "Closing down wolfSSHD\n");
208206
quit = 1;
209207
}
210208

@@ -2762,6 +2760,9 @@ static int StartSSHD(int argc, char** argv)
27622760
}
27632761
#endif
27642762
}
2763+
if (quit && logFile) {
2764+
fprintf(logFile, "Closing down wolfSSHD\n");
2765+
}
27652766
}
27662767

27672768
#ifdef _WIN32

‎examples/scpclient/scpclient.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,6 @@
2222
#ifndef _WOLFSSH_EXAMPLES_SCPCLIENT_H_
2323
#define _WOLFSSH_EXAMPLES_SCPCLIENT_H_
2424

25-
THREAD_RETURN WOLFSSH_THREAD scp_client(void*);
25+
THREAD_RETURN WOLFSSH_THREAD scp_client(void* args);
2626

2727
#endif /* _WOLFSSH_EXAMPLES_SCPCLIENT_H_ */

‎examples/sftpclient/sftpclient.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static void err_msg(const char* s)
105105
static word32 startTime;
106106
#define TIMEOUT_VALUE 120
107107

108-
word32 current_time(int);
108+
word32 current_time(int reset);
109109
#ifdef USE_WINDOWS_API
110110
#include <time.h>
111111
#define WIN32_LEAN_AND_MEAN
@@ -268,7 +268,7 @@ static void sig_handler(const int sig)
268268
(void)sig;
269269

270270
interrupt = 1;
271-
wolfSSH_SFTP_Interrupt(ssh);
271+
wolfSSH_SFTP_Interrupt(ssh); /* NOLINT(bugprone-signal-handler) */
272272
}
273273
#endif /* WS_NO_SIGNAL */
274274

‎src/internal.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7563,7 +7563,6 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
75637563
publicKeyTypeSz) == 0) {
75647564
sigTypeOk = 1;
75657565
}
7566-
#ifdef WOLFSSH_CERTS
75677566
else if (publicKeyType != NULL
75687567
&& pk->publicKeyTypeSz == 14
75697568
&& WMEMCMP(pk->publicKeyType,
@@ -7581,7 +7580,6 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
75817580
sigTypeOk = 1;
75827581
}
75837582
}
7584-
#endif
75857583
if (!sigTypeOk) {
75867584
WLOG(WS_LOG_DEBUG,
75877585
"Signature's type does not match public key type");

‎src/log.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@
5050

5151

5252
#ifndef WOLFSSL_NO_DEFAULT_LOGGING_CB
53-
static void DefaultLoggingCb(enum wolfSSH_LogLevel, const char *const);
53+
static void DefaultLoggingCb(enum wolfSSH_LogLevel level,
54+
const char *const msgStr);
5455
static wolfSSH_LoggingCb logFunction = DefaultLoggingCb;
5556
#else /* WOLFSSH_NO_DEFAULT_LOGGING_CB */
5657
static wolfSSH_LoggingCb logFunction = NULL;

‎src/wolfscp.c‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,12 +1078,19 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz,
10781078

10791079
/* skip '0 ' */
10801080
if (ret == WS_SUCCESS) {
1081-
if (buf[idx] != '0' || ++idx > bufSz)
1081+
if (buf[idx] != '0') {
10821082
ret = WS_SCP_TIMESTAMP_E;
1083-
1084-
if (ret == WS_SUCCESS) {
1085-
if (buf[idx] != ' ' || ++idx > bufSz)
1083+
}
1084+
else {
1085+
idx++;
1086+
if (idx >= bufSz || buf[idx] != ' ') {
10861087
ret = WS_SCP_TIMESTAMP_E;
1088+
}
1089+
else {
1090+
idx++;
1091+
if (idx >= bufSz)
1092+
ret = WS_SCP_TIMESTAMP_E;
1093+
}
10871094
}
10881095
}
10891096

0 commit comments

Comments
 (0)