diff --git a/common/list.c b/common/list.c index c7528052..463c5e2d 100644 --- a/common/list.c +++ b/common/list.c @@ -271,7 +271,7 @@ list_add_strdup_multi(struct list *self, ...) int rv = 1; va_start(ap, self); - while ((s = va_arg(ap, const char *)) != NULL) + while ((s = va_arg(ap, const char *)) != LIST_ADD_STRDUP_TERM) { if (!list_add_strdup(self, s)) { diff --git a/common/list.h b/common/list.h index dfefa439..288d92b9 100644 --- a/common/list.h +++ b/common/list.h @@ -149,12 +149,16 @@ list_add_strdup(struct list *self, const char *str); * * This is a convenience function for a common operation * @param self List to append to - * @param ... Strings to append. Terminate the list with a NULL. + * @param ... Strings to append. Terminate the list with LIST_ADD_STRDUP_TERM * * @result 0 if any memory allocation failure occurred. In this case * the list is unchanged. */ +/* + * We need a typed terminator to guarantee the stack object is the + * correct size (cf C99 std 6.2.5(12) for static checkers */ +#define LIST_ADD_STRDUP_TERM ((const char *)0) int list_add_strdup_multi(struct list *self, ...); diff --git a/scripts/run_cppcheck.sh b/scripts/run_cppcheck.sh index 16d32135..1ed15547 100755 --- a/scripts/run_cppcheck.sh +++ b/scripts/run_cppcheck.sh @@ -84,7 +84,7 @@ fi # Supply default flags passed to cppcheck if necessary if [ -z "$CPPCHECK_FLAGS" ]; then CPPCHECK_FLAGS="--quiet --force --std=c11 --std=c++11 --inline-suppr \ - --enable=warning,missingInclude,style --disable=portability --disable=performance --error-exitcode=1 \ + --enable=warning,missingInclude,style,portability --disable=performance --error-exitcode=1 \ -i third_party \ -i vrplayer \ --suppress=constParameterCallback --suppress=variableScope --suppress=constParameterPointer --suppress=constVariablePointer --suppress=duplicateCondition --suppress=redundantAssignment --suppress=knownConditionTrueFalse --suppress=unreadVariable --suppress=constParameter --suppress=constVariable --suppress=shadowFunction --suppress=unusedStructMember --suppress=duplicateExpression --suppress=unsignedLessThanZero --suppress=shadowVariable --suppress=unreachableCode --suppress=noExplicitConstructor --suppress=constParameterReference --suppress=functionStatic --suppress=shadowArgument --suppress=truncLongCastAssignment --suppress=cstyleCast --suppress=constVariableReference --suppress=badBitmaskCheck \ diff --git a/sesman/libsesman/verify_user.c b/sesman/libsesman/verify_user.c index fad83b42..8251e634 100644 --- a/sesman/libsesman/verify_user.c +++ b/sesman/libsesman/verify_user.c @@ -45,8 +45,8 @@ #define SECS_PER_DAY (24L*3600L) #endif -static int -auth_crypt_pwd(const char *pwd, const char *pln, char *crp); +static char * +auth_crypt_pwd(const char *pwd, const char *pln); static int auth_account_disabled(struct spwd *stp); @@ -257,7 +257,7 @@ auth_change_pwd(const char *user, const char *newpwd) { struct passwd *spw; struct spwd *stp; - char hash[35] = ""; + char *newpw; long today; FILE *fd; @@ -278,13 +278,13 @@ auth_change_pwd(const char *user, const char *newpwd) if (g_strncmp(spw->pw_passwd, "x", 3) != 0) { /* old system with only passwd */ - if (auth_crypt_pwd(spw->pw_passwd, newpwd, hash) != 0) + if ((newpw = auth_crypt_pwd(spw->pw_passwd, newpwd)) == NULL) { ulckpwdf(); return 1; } - spw->pw_passwd = g_strdup(hash); + spw->pw_passwd = newpw; fd = fopen("/etc/passwd", "rw"); putpwent(spw, fd); } @@ -299,13 +299,13 @@ auth_change_pwd(const char *user, const char *newpwd) } /* old system with only passwd */ - if (auth_crypt_pwd(stp->sp_pwdp, newpwd, hash) != 0) + if ((newpw = auth_crypt_pwd(stp->sp_pwdp, newpwd)) == NULL) { ulckpwdf(); return 1; } - stp->sp_pwdp = g_strdup(hash); + stp->sp_pwdp = newpw; today = time(NULL) / SECS_PER_DAY; stp->sp_lstchg = today; stp->sp_expire = today + stp->sp_max + stp->sp_inact; @@ -322,43 +322,21 @@ auth_change_pwd(const char *user, const char *newpwd) * @brief Password encryption * @param pwd Old password * @param pln Plaintext new password - * @param crp Crypted new password - * + * @return Dynamically allocated new password */ -static int -auth_crypt_pwd(const char *pwd, const char *pln, char *crp) +static char * +auth_crypt_pwd(const char *pwd, const char *pln) { - char salt[13] = "$1$"; - int saltcnt = 0; - char *encr; + char random[32]; - if (g_strncmp(pwd, "$1$", 3) == 0) - { - /* gnu style crypt(); */ - saltcnt = 3; + g_random(random, sizeof(random)); - while ((pwd[saltcnt] != '$') && (saltcnt < 11)) - { - salt[saltcnt] = pwd[saltcnt]; - saltcnt++; - } - - salt[saltcnt] = '$'; - salt[saltcnt + 1] = '\0'; - } - else - { - /* classic two char salt */ - salt[0] = pwd[0]; - salt[1] = pwd[1]; - salt[2] = '\0'; - } - - encr = crypt(pln, salt); - g_strncpy(crp, encr, 34); - - return 0; + // crypt_gensalt() is not defined by POSIX, but we have to change + // the salt when the password is changed. + const char *encr = crypt(pln, + crypt_gensalt(pwd, 0, random, sizeof(random))); + return g_strdup(encr); } /** diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 8ce465c3..d69c76e1 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -399,7 +399,7 @@ prepare_xorg_xserver_params(const struct session_parameters *s, list_add_strdup_multi(params, xserver, screen, "-auth", authfile, - NULL); + LIST_ADD_STRDUP_TERM); /* additional parameters from sesman.ini file */ list_append_list_strdup(g_cfg->xorg_params, params, 1); @@ -451,7 +451,7 @@ prepare_xvnc_xserver_params(const struct session_parameters *s, "-auth", authfile, "-geometry", geometry, "-depth", depth, - NULL); + LIST_ADD_STRDUP_TERM); if (passwd_file != NULL) { @@ -459,7 +459,7 @@ prepare_xvnc_xserver_params(const struct session_parameters *s, env_check_password_file(passwd_file, guid_str); list_add_strdup_multi(params, "-rfbauth", passwd_file, - NULL); + LIST_ADD_STRDUP_TERM); } else if (port != NULL) { @@ -478,7 +478,7 @@ prepare_xvnc_xserver_params(const struct session_parameters *s, "-rfbunixpath", port, "-rfbunixmode", sock_mode, "-SecurityTypes", "None", - NULL); + LIST_ADD_STRDUP_TERM); } /* additional parameters from sesman.ini file */ diff --git a/sesman/sesexec/xwait.c b/sesman/sesexec/xwait.c index 7531d451..93cfca04 100644 --- a/sesman/sesexec/xwait.c +++ b/sesman/sesexec/xwait.c @@ -67,7 +67,8 @@ make_xwait_command(int display) cmd->auto_free = 1; g_snprintf(displaystr, sizeof(displaystr), ":%d", display); - if (!list_add_strdup_multi(cmd, exe, "-d", displaystr, NULL)) + if (!list_add_strdup_multi(cmd, exe, "-d", displaystr, + LIST_ADD_STRDUP_TERM)) { list_delete(cmd); cmd = NULL; diff --git a/sesman/sesexec_control.c b/sesman/sesexec_control.c index ee0684e9..06b2eed7 100644 --- a/sesman/sesexec_control.c +++ b/sesman/sesexec_control.c @@ -69,7 +69,8 @@ create_exec_args_add_entries(struct list *args) if (g_strcmp(g_cfg->sesman_ini, DEFAULT_SESMAN_INI) != 0) { - if (!list_add_strdup_multi(args, "-c", g_cfg->sesman_ini, NULL)) + if (!list_add_strdup_multi(args, "-c", g_cfg->sesman_ini, + LIST_ADD_STRDUP_TERM)) { return 0; } diff --git a/sesman/sesman.c b/sesman/sesman.c index fc7ad8c8..d940b29a 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -152,32 +152,33 @@ sesman_process_params(int argc, char **argv, value = ""; } - if (nocase_matches(option, "-help", "--help", "-h", NULL)) + if (nocase_matches(option, "-help", "--help", "-h", (const char *)0)) { startup_params->help = 1; } - else if (nocase_matches(option, "-kill", "--kill", "-k", NULL)) + else if (nocase_matches(option, "-kill", "--kill", "-k", + (const char *)0)) { startup_params->mode = SSM_KILL_DAEMON; } - else if (nocase_matches(option, "-reload", "--reload", "-r", NULL)) + else if (nocase_matches(option, "-reload", "--reload", "-r", (const char *)0)) { startup_params->mode = SSM_RELOAD_DAEMON; } else if (nocase_matches(option, "-nodaemon", "--nodaemon", "-n", - "-nd", "--nd", "-ns", "--ns", NULL)) + "-nd", "--nd", "-ns", "--ns", (const char *)0)) { startup_params->no_daemon = 1; } - else if (nocase_matches(option, "-v", "--version", NULL)) + else if (nocase_matches(option, "-v", "--version", (const char *)0)) { startup_params->version = 1; } - else if (nocase_matches(option, "--dump-config", NULL)) + else if (nocase_matches(option, "--dump-config", (const char *)0)) { startup_params->dump_config = 1; } - else if (nocase_matches(option, "-c", "--config", NULL)) + else if (nocase_matches(option, "-c", "--config", (const char *)0)) { index++; startup_params->sesman_ini = value; diff --git a/tests/common/test_list_calls.c b/tests/common/test_list_calls.c index 63d83966..391cef7f 100644 --- a/tests/common/test_list_calls.c +++ b/tests/common/test_list_calls.c @@ -133,7 +133,7 @@ START_TEST(test_list__simple_strdup_multi) list_add_strdup_multi(lst, "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", - NULL); + LIST_ADD_STRDUP_TERM); ck_assert_int_eq(lst->count, 12); diff --git a/tests/libipm/test_libipm_recv_calls.c b/tests/libipm/test_libipm_recv_calls.c index 6129be03..fa29a099 100644 --- a/tests/libipm/test_libipm_recv_calls.c +++ b/tests/libipm/test_libipm_recv_calls.c @@ -742,8 +742,8 @@ START_TEST(test_libipm_receive_unsupported_type) *g_t_in->in_s->p = 'A'; /* unsupported type */ c = libipm_msg_in_peek_type(g_t_in); ck_assert_int_eq(c, '?'); /* peek should say this is an error */ - - status = libipm_msg_in_parse( g_t_in, "A", NULL); /* Parse it anyway */ + /* Parse it anyway */ + status = libipm_msg_in_parse( g_t_in, "A", (const char *)0); ck_assert_int_eq(status, E_LI_UNSUPPORTED_TYPE); } END_TEST @@ -783,7 +783,7 @@ START_TEST(test_libipm_receive_unimplemented_type) *g_t_in->in_s->p = 'd'; /* reserved type */ c = libipm_msg_in_peek_type(g_t_in); ck_assert_int_eq(c, 'd'); - status = libipm_msg_in_parse( g_t_in, "d", NULL); + status = libipm_msg_in_parse( g_t_in, "d", (const char *)0); ck_assert_int_eq(status, E_LI_UNIMPLEMENTED_TYPE); } END_TEST diff --git a/tests/libipm/test_libipm_send_calls.c b/tests/libipm/test_libipm_send_calls.c index 34cd2cb0..8c225def 100644 --- a/tests/libipm/test_libipm_send_calls.c +++ b/tests/libipm/test_libipm_send_calls.c @@ -349,7 +349,8 @@ START_TEST(test_libipm_send_s_type) ck_assert_int_eq(status, E_LI_SUCCESS); /* Check passing a NULL string doesn't crash the program */ - status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO, "s", NULL); + status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO, + "s", (const char *)0); ck_assert_int_eq(status, E_LI_PROGRAM_ERROR); } END_TEST @@ -409,7 +410,8 @@ START_TEST(test_libipm_send_B_type) desc.data = NULL; status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO, "B", &desc); ck_assert_int_eq(status, E_LI_PROGRAM_ERROR); - status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO, "B", NULL); + status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO, "B", + (const struct libipm_fsb *)0); ck_assert_int_eq(status, E_LI_PROGRAM_ERROR); } END_TEST @@ -447,7 +449,7 @@ START_TEST(test_libipm_send_bad_types) format[0] = c; status = libipm_msg_out_init(g_t_out, TEST_MESSAGE_NO_STRING_NO, - format, NULL); + format, (const char *)0); if (status != expected_status) { ck_abort_msg("Output char '%c'. Expected status %d, got %d", diff --git a/xrdp/xrdp.c b/xrdp/xrdp.c index b414b7b9..b11b3f5e 100644 --- a/xrdp/xrdp.c +++ b/xrdp/xrdp.c @@ -177,24 +177,25 @@ xrdp_process_params(int argc, char **argv, value = ""; } - if (nocase_matches(option, "-help", "--help", "-h", NULL)) + if (nocase_matches(option, "-help", "--help", "-h", (const char *)0)) { startup_params->help = 1; } - else if (nocase_matches(option, "-kill", "--kill", "-k", NULL)) + else if (nocase_matches(option, "-kill", "--kill", "-k", + (const char *)0)) { startup_params->kill = 1; } else if (nocase_matches(option, "-nodaemon", "--nodaemon", "-n", - "-nd", "--nd", "-ns", "--ns", NULL)) + "-nd", "--nd", "-ns", "--ns", (const char *)0)) { startup_params->no_daemon = 1; } - else if (nocase_matches(option, "-v", "--version", NULL)) + else if (nocase_matches(option, "-v", "--version", (const char *)0)) { startup_params->version = 1; } - else if (nocase_matches(option, "-p", "--port", NULL)) + else if (nocase_matches(option, "-p", "--port", (const char *)0)) { index++; g_strncpy(startup_params->port, value, @@ -211,20 +212,20 @@ xrdp_process_params(int argc, char **argv, startup_params->port); } } - else if (nocase_matches(option, "-f", "--fork", NULL)) + else if (nocase_matches(option, "-f", "--fork", (const char *)0)) { startup_params->fork = 1; g_writeln("--fork parameter found, ini override"); } - else if (nocase_matches(option, "--dump-config", NULL)) + else if (nocase_matches(option, "--dump-config", (const char *)0)) { startup_params->dump_config = 1; } - else if (nocase_matches(option, "--license", NULL)) + else if (nocase_matches(option, "--license", (const char *)0)) { startup_params->license = 1; } - else if (nocase_matches(option, "-c", "--config", NULL)) + else if (nocase_matches(option, "-c", "--config", (const char *)0)) { index++; startup_params->xrdp_ini = value; diff --git a/xrdp/xrdp_bitmap.c b/xrdp/xrdp_bitmap.c index 80a5dc1e..1d9d9492 100644 --- a/xrdp/xrdp_bitmap.c +++ b/xrdp/xrdp_bitmap.c @@ -160,7 +160,7 @@ xrdp_bitmap_hash_crc(struct xrdp_bitmap *self) { void *hash; int bytes; - int crc; + unsigned int crc; int index; char hash_data[16]; @@ -213,7 +213,7 @@ xrdp_bitmap_copy_box_with_crc(struct xrdp_bitmap *self, int destx; int desty; int pixel; - int crc; + unsigned int crc; int incs; int incd; tui8 *s8; diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index 858c8e48..6be9b93d 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -713,7 +713,7 @@ struct xrdp_bitmap struct xrdp_bitmap *popped_from; int item_height; /* crc */ - int crc32; + unsigned int crc32; int crc16; };