diff options
37 files changed, 1151 insertions, 415 deletions
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..c4b9473 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,45 @@ +name: "CodeQL" + +on: + push: + branches: ["main"] + pull_request: + # The branches below must be a subset of the branches above + branches: ["main"] + schedule: + - cron: '29 1 * * 3' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: ['cpp'] + + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + queries: security-extended,security-and-quality + + - run: | + sudo apt-get update + sudo apt-get install libbsd-dev + make + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{matrix.language}}" diff --git a/CHANGELOG.md b/CHANGELOG.md index cc96d7a..1c09a38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,39 @@ All notable changes to this fork of FICS version 1.6.2 will be documented in this file. ## [Unreleased] ## +- Fixed multiplication result converted to larger type. Two + occurrences. +- Replaced non-reentrant functions with their corresponding thread + safe version. (Multiple occurrences, found by CodeQL.) + +## [1.4.5] - 2025-04-09 ## +- **Changed** the addplayer program to output a restart notice if an admin + account is created. +- **Changed** the program to avoid calculating the same string multiple + times. Multiple occurrences, found by PVS-Studio. +- **Fixed** `-Wshadow` warnings. Multiple occurrences. +- **Fixed** calls of risky functions. +- **Fixed** constant expression result in `tell()`. +- **Fixed** double `free()` in `process_login()`. +- **Fixed** excessive checks. +- **Fixed** improper use of negative values. +- **Fixed** memory leak in `process_login()`. +- **Fixed** _multiple_ Clang Tidy warnings. +- **Fixed** negative array index read in `accept_match()`. +- **Fixed** null pointer dereferences. +- **Fixed** out-of-bounds array access in `has_legal_move()`. +- **Fixed** overflowed array index read/write. Multiple occurrences. +- **Fixed** overflowed return value in `player_search()`. +- **Fixed** possible buffer overflow in `FindHistory2()`. +- **Fixed** possible memory corruptions and incorrect computations. +- **Fixed** truncated stdio return value in `ReadGameState()`. +- **Fixed** unchecked function return values. Multiple occurrences. +- **Fixed** uninitialized variables. +- **Fixed** untrusted array indices. +- **Fixed** untrusted loop bounds. +- **Fixed** use of 32-bit `time_t`. Y2K38 safety. Multiple occurrences. + +## [1.4.4] - 2024-12-07 ## - **Added** an autorun script suitable to be run as a cron job. - **Added** command `sought`, which currently behaves as a no-op. Code is to be added in a later version. We also want the seek/unseek diff --git a/FICS/ISC_LICENSE b/FICS/ISC_LICENSE index eba7139..915b183 100644 --- a/FICS/ISC_LICENSE +++ b/FICS/ISC_LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2023, 2024 Markus Uhlin <maxxe@rpblc.net> +Copyright (c) 2023 - 2025 Markus Uhlin <maxxe@rpblc.net> Permission to use, copy, modify, and distribute this software for any purpose with or without fee is hereby granted, provided that the above diff --git a/FICS/adminproc.c b/FICS/adminproc.c index bbb0c24..04331a4 100644 --- a/FICS/adminproc.c +++ b/FICS/adminproc.c @@ -1,17 +1,17 @@ /* - * fics - An internet chess server - * Copyright (c) 1993 Richard V. Nash - * - * This program is free software; you can redistribute it and/or modify it under - * the terms of the GNU General Public License as published by the Free Software - * Foundation; either version 2 of the License, or (at your option) any later - * version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS - * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more - * details. - */ + fics - An internet chess server. + Copyright (C) 1993 Richard V. Nash + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. +*/ #include "stdinclude.h" #include "common.h" @@ -183,9 +183,10 @@ create_news_file(int p, param_list param, int admin) msnprintf(filename, sizeof filename, "%s/adminnews.%d", news_dir, param[0].val.integer); - fp = fopen(filename, "w"); - fprintf(fp, "%s\n", param[1].val.string); - fclose(fp); + if ((fp = fopen(filename, "w")) != NULL) { + fprintf(fp, "%s\n", param[1].val.string); + fclose(fp); + } } } else { if (param[0].val.integer > num_news) { @@ -195,9 +196,10 @@ create_news_file(int p, param_list param, int admin) msnprintf(filename, sizeof filename, "%s/news.%d", news_dir, param[0].val.integer); - fp = fopen(filename, "w"); - fprintf(fp, "%s\n", param[1].val.string); - fclose(fp); + if ((fp = fopen(filename, "w")) != NULL) { + fprintf(fp, "%s\n", param[1].val.string); + fclose(fp); + } } } @@ -228,13 +230,14 @@ add_item(char *new_item, char *filename) } end: - fclose(new_fp); + (void) fclose(new_fp); + if (old_fp) { - fclose(old_fp); - remove(filename); + (void) fclose(old_fp); + (void) remove(filename); } - rename(tmp_file, filename); + (void) rename(tmp_file, filename); return 1; } @@ -499,9 +502,15 @@ com_anews(int p, param_list param) PUBLIC int strcmpwild(char *mainstr, char *searchstr) { - if (strlen(mainstr) < strlen(searchstr)) + size_t len[2]; + + len[0] = strlen(mainstr); + len[1] = strlen(searchstr); + + if (len[0] < len[1]) return 1; - for (size_t i = 0; i < strlen(mainstr); i++) { + + for (size_t i = 0; i < len[0]; i++) { if (searchstr[i] == '*') return 0; if (mainstr[i] != searchstr[i]) @@ -589,7 +598,7 @@ com_checkSOCKET(int p, param_list param) PUBLIC int com_checkPLAYER(int p, param_list param) { - char *player = param[0].val.word; + char *v_player = param[0].val.word; int p1; ASSERT(parray[p].adminLevel >= ADMIN_ADMIN); @@ -601,48 +610,60 @@ com_checkPLAYER(int p, param_list param) if (p1 < 0) { p1 = (-p1) - 1; - pprintf(p, "%s is not logged in.\n", player); - stolower(player); - - pprintf(p, "name = %s\n", parray[p1].name); - pprintf(p, "login = %s\n", parray[p1].login); - pprintf(p, "fullName = %s\n", (parray[p1].fullName ? - parray[p1].fullName : "(none)")); - pprintf(p, "emailAddress = %s\n", (parray[p1].emailAddress ? - parray[p1].emailAddress : "(none)")); - pprintf(p, "adminLevel = %d\n", parray[p1].adminLevel); + pprintf(p, "%s is not logged in.\n", v_player); + stolower(v_player); + + pprintf(p, "name = %s\n", parray[p1].name); + pprintf(p, "login = %s\n", parray[p1].login); + pprintf(p, "fullName = %s\n", + (parray[p1].fullName + ? parray[p1].fullName + : "(none)")); + pprintf(p, "emailAddress = %s\n", + (parray[p1].emailAddress + ? parray[p1].emailAddress + : "(none)")); + pprintf(p, "adminLevel = %d\n", parray[p1].adminLevel); #if 0 pprintf(p, "network_player = %d\n", parray[p1].network_player); #endif - pprintf(p, "lastHost = %s\n", dotQuad(parray[p1].lastHost)); - pprintf(p, "num_comments = %d\n", parray[p1].num_comments); + pprintf(p, "lastHost = %s\n", dotQuad(parray[p1].lastHost)); + pprintf(p, "num_comments = %d\n", parray[p1].num_comments); player_remove(p1); return COM_OK; } else { + char tbuf[30] = { '\0' }; + p1 = p1 - 1; - pprintf(p, "%s is number %d in parray of size %d\n", player, p1, + pprintf(p, "%s is number %d in parray of size %d\n", v_player, p1, (p_num + 1)); - pprintf(p, "name = %s\n", parray[p1].name); - pprintf(p, "login = %s\n", parray[p1].login); - pprintf(p, "fullName = %s\n", (parray[p1].fullName ? - parray[p1].fullName : "(none)")); - pprintf(p, "emailAddress = %s\n", (parray[p1].emailAddress ? - parray[p1].emailAddress : "(none)")); - pprintf(p, "socket = %d\n", parray[p1].socket); - pprintf(p, "registered = %d\n", parray[p1].registered); - pprintf(p, "last_tell = %d\n", parray[p1].last_tell); - pprintf(p, "last_channel = %d\n", parray[p1].last_channel); - pprintf(p, "logon_time = %s", - ctime((time_t *) &parray[p1].logon_time)); - pprintf(p, "adminLevel = %d\n", parray[p1].adminLevel); + pprintf(p, "name = %s\n", parray[p1].name); + pprintf(p, "login = %s\n", parray[p1].login); + pprintf(p, "fullName = %s\n", + (parray[p1].fullName + ? parray[p1].fullName + : "(none)")); + pprintf(p, "emailAddress = %s\n", + (parray[p1].emailAddress + ? parray[p1].emailAddress + : "(none)")); + pprintf(p, "socket = %d\n", parray[p1].socket); + pprintf(p, "registered = %d\n", parray[p1].registered); + pprintf(p, "last_tell = %d\n", parray[p1].last_tell); + pprintf(p, "last_channel = %d\n", parray[p1].last_channel); + pprintf(p, "logon_time = %s", + (ctime_r(&parray[p1].logon_time, tbuf) != NULL + ? &tbuf[0] + : "n/a")); + pprintf(p, "adminLevel = %d\n", parray[p1].adminLevel); #if 0 pprintf(p, "network_player = %d\n", parray[p1].network_player); #endif - pprintf(p, "thisHost = %s\n", dotQuad(parray[p1].thisHost)); - pprintf(p, "lastHost = %s\n", dotQuad(parray[p1].lastHost)); - pprintf(p, "num_comments = %d\n", parray[p1].num_comments); + pprintf(p, "thisHost = %s\n", dotQuad(parray[p1].thisHost)); + pprintf(p, "lastHost = %s\n", dotQuad(parray[p1].lastHost)); + pprintf(p, "num_comments = %d\n", parray[p1].num_comments); } return COM_OK; @@ -713,7 +734,8 @@ com_checkGAME(int p, param_list param) // than that for (g = 0; g < g_num; g++) { - multicol_store(m, tmp); + memset(tmp, 0, sizeof tmp); // XXX + multicol_store(m, tmp); // is this call right? if (!strcasecmp(garray[g].white_name, param[0].val.word)) { @@ -877,13 +899,13 @@ com_checkGAME(int p, param_list param) PUBLIC int com_remplayer(int p, param_list param) { - char *player = param[0].val.word; + char *v_player = param[0].val.word; char playerlower[MAX_LOGIN_NAME] = { '\0' }; int p1, lookup; ASSERT(parray[p].adminLevel >= ADMIN_ADMIN); - mstrlcpy(playerlower, player, sizeof(playerlower)); + mstrlcpy(playerlower, v_player, sizeof(playerlower)); stolower(playerlower); p1 = player_new(); lookup = player_read(p1, playerlower); @@ -901,7 +923,7 @@ com_remplayer(int p, param_list param) player_remove(p1); if (lookup) { - pprintf(p, "No player by the name %s is registered.\n", player); + pprintf(p, "No player by the name %s is registered.\n", v_player); return COM_OK; } @@ -911,10 +933,10 @@ com_remplayer(int p, param_list param) } if (!player_kill(playerlower)) { - pprintf(p, "Player %s removed.\n", player); - UpdateRank(TYPE_BLITZ, NULL, NULL, player); - UpdateRank(TYPE_STAND, NULL, NULL, player); - UpdateRank(TYPE_WILD, NULL, NULL, player); + pprintf(p, "Player %s removed.\n", v_player); + UpdateRank(TYPE_BLITZ, NULL, NULL, v_player); + UpdateRank(TYPE_STAND, NULL, NULL, v_player); + UpdateRank(TYPE_WILD, NULL, NULL, v_player); } else { pprintf(p, "Remplayer failed.\n"); } @@ -939,14 +961,14 @@ com_remplayer(int p, param_list param) PUBLIC int com_raisedead(int p, param_list param) { - char *player = param[0].val.word; + char *v_player = param[0].val.word; char newplayerlower[MAX_LOGIN_NAME] = { '\0' }; char playerlower[MAX_LOGIN_NAME] = { '\0' }; int p1, p2, lookup; ASSERT(parray[p].adminLevel >= ADMIN_ADMIN); - mstrlcpy(playerlower, player, sizeof playerlower); + mstrlcpy(playerlower, v_player, sizeof playerlower); stolower(playerlower); if (player_find_bylogin(playerlower) >= 0) { @@ -961,7 +983,7 @@ com_raisedead(int p, param_list param) if (!lookup) { pprintf(p, "A player by the name %s is already registered.\n", - player); + v_player); pprintf(p, "Obtain a new handle for the dead person.\n"); pprintf(p, "Then use raisedead [oldname] [newname].\n"); return COM_OK; @@ -969,22 +991,22 @@ com_raisedead(int p, param_list param) if (param[1].type == TYPE_NULL) { if (!player_raise(playerlower)) { - pprintf(p, "Player %s raised from dead.\n", player); + pprintf(p, "Player %s raised from dead.\n", v_player); p1 = player_new(); if (!player_read(p1, playerlower)) { if (parray[p1].s_stats.rating > 0) { - UpdateRank(TYPE_STAND, player, - &parray[p1].s_stats, player); + UpdateRank(TYPE_STAND, v_player, + &parray[p1].s_stats, v_player); } if (parray[p1].b_stats.rating > 0) { - UpdateRank(TYPE_BLITZ, player, - &parray[p1].b_stats, player); + UpdateRank(TYPE_BLITZ, v_player, + &parray[p1].b_stats, v_player); } if (parray[p1].w_stats.rating > 0) { - UpdateRank(TYPE_WILD, player, - &parray[p1].w_stats, player); + UpdateRank(TYPE_WILD, v_player, + &parray[p1].w_stats, v_player); } } @@ -1013,14 +1035,14 @@ com_raisedead(int p, param_list param) if (!lookup) { pprintf(p, "A player by the name %s is already " - "registered.\n", player); + "registered.\n", v_player); pprintf(p, "Obtain another new handle for the dead " "person.\n"); return COM_OK; } if (!player_reincarn(playerlower, newplayerlower)) { - pprintf(p, "Player %s reincarnated to %s.\n", player, + pprintf(p, "Player %s reincarnated to %s.\n", v_player, newplayer); p2 = player_new(); @@ -1543,14 +1565,14 @@ PUBLIC int com_asethandle(int p, param_list param) { char *newplayer = param[1].val.word; - char *player = param[0].val.word; + char *v_player = param[0].val.word; char newplayerlower[MAX_LOGIN_NAME] = { '\0' }; char playerlower[MAX_LOGIN_NAME] = { '\0' }; int p1; ASSERT(parray[p].adminLevel >= ADMIN_ADMIN); - mstrlcpy(playerlower, player, sizeof playerlower); + mstrlcpy(playerlower, v_player, sizeof playerlower); stolower(playerlower); mstrlcpy(newplayerlower, newplayer, sizeof newplayerlower); @@ -1569,7 +1591,7 @@ com_asethandle(int p, param_list param) p1 = player_new(); if (player_read(p1, playerlower)) { - pprintf(p, "No player by the name %s is registered.\n", player); + pprintf(p, "No player by the name %s is registered.\n", v_player); player_remove(p1); return COM_OK; } else { @@ -1596,22 +1618,22 @@ com_asethandle(int p, param_list param) if ((!player_rename(playerlower, newplayerlower)) && (!player_read(p1, newplayerlower))) { - pprintf(p, "Player %s renamed to %s.\n", player, newplayer); + pprintf(p, "Player %s renamed to %s.\n", v_player, newplayer); strfree(parray[p1].name); parray[p1].name = xstrdup(newplayer); player_save(p1); if (parray[p1].s_stats.rating > 0) { UpdateRank(TYPE_STAND, newplayer, &parray[p1].s_stats, - player); + v_player); } if (parray[p1].b_stats.rating > 0) { UpdateRank(TYPE_BLITZ, newplayer, &parray[p1].b_stats, - player); + v_player); } if (parray[p1].w_stats.rating > 0) { UpdateRank(TYPE_WILD, newplayer, &parray[p1].w_stats, - player); + v_player); } } else { pprintf(p, "Asethandle failed.\n"); diff --git a/FICS/algcheck.c b/FICS/algcheck.c index 250c129..15642fd 100644 --- a/FICS/algcheck.c +++ b/FICS/algcheck.c @@ -21,11 +21,16 @@ name email yy/mm/dd Change Richard Nash 93/10/22 Created Markus Uhlin 24/05/05 Revised + Markus Uhlin 25/04/05 alg_parse_move: + return ambiguous move on + out-of-bounds array read/write. */ #include "stdinclude.h" #include "common.h" +#include <err.h> + #include "algcheck.h" #include "board.h" #include "maxxes-utils.h" @@ -250,6 +255,11 @@ alg_parse_move(char *mstr, game_state_t *gs, move_t *mt) NextPieceLoop(gs->board, &f, &r, gs->onMove);) { if ((ff != ALG_UNKNOWN) && (ff != f)) continue; + if (r < 0 || r >= 8) { + warnx("%s: out-of-bounds array read/write: " + "r=%d", __func__, r); + return MOVE_AMBIGUOUS; + } if (piecetype(gs->board[f][r]) != piece) continue; if (gs->onMove == WHITE) { @@ -257,6 +267,11 @@ alg_parse_move(char *mstr, game_state_t *gs, move_t *mt) } else { tmpr = r - 1; } + if (tmpr < 0 || tmpr >= 8) { + warnx("%s: out-of-bounds array read/write: " + "tmpr=%d", __func__, tmpr); + return MOVE_AMBIGUOUS; + } if (gs->board[tf][tmpr] == NOPIECE) { if ((gs->ep_possible[((gs->onMove == WHITE) ? diff --git a/FICS/board.c b/FICS/board.c index f687b3e..d0b65a2 100644 --- a/FICS/board.c +++ b/FICS/board.c @@ -25,6 +25,7 @@ Markus Uhlin 24/04/13 Added usage of the functions from 'maxxes-utils.h'. Markus Uhlin 24/06/01 Added and made use of brand(). + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" @@ -1133,7 +1134,7 @@ board_read_file(char *category, char *gname, game_state_t *gs) case 'g': case 'h': onFile = (c - 'a'); - onRank = -1; + onRank = -1; // NOLINT: dead store break; case '1': case '2': @@ -1161,7 +1162,7 @@ board_read_file(char *category, char *gname, game_state_t *gs) onColor = -1; onPiece = -1; onFile = -1; - onRank = -1; + onRank = -1; // NOLINT: dead store break; default: break; diff --git a/FICS/build.mk b/FICS/build.mk index f8c7d05..834ebef 100644 --- a/FICS/build.mk +++ b/FICS/build.mk @@ -37,6 +37,40 @@ OBJS = $(SRC_DIR)adminproc.o\ $(SRC_DIR)variable.o\ $(SRC_DIR)vers.o +SRCS = $(SRC_DIR)adminproc.c\ + $(SRC_DIR)algcheck.c\ + $(SRC_DIR)assert_error.c\ + $(SRC_DIR)board.c\ + $(SRC_DIR)command.c\ + $(SRC_DIR)comproc.c\ + $(SRC_DIR)eco.c\ + $(SRC_DIR)fics_getsalt.cpp\ + $(SRC_DIR)ficslim.cpp\ + $(SRC_DIR)ficsmain.c\ + $(SRC_DIR)formula.c\ + $(SRC_DIR)gamedb.c\ + $(SRC_DIR)gameproc.c\ + $(SRC_DIR)iset.cpp\ + $(SRC_DIR)legal.c\ + $(SRC_DIR)legal2.c\ + $(SRC_DIR)lists.c\ + $(SRC_DIR)matchproc.c\ + $(SRC_DIR)maxxes-utils.c\ + $(SRC_DIR)movecheck.c\ + $(SRC_DIR)multicol.c\ + $(SRC_DIR)network.c\ + $(SRC_DIR)obsproc.c\ + $(SRC_DIR)playerdb.c\ + $(SRC_DIR)rating_conv.c\ + $(SRC_DIR)ratings.c\ + $(SRC_DIR)rmalloc.c\ + $(SRC_DIR)shutdown.c\ + $(SRC_DIR)sought.cpp\ + $(SRC_DIR)talkproc.c\ + $(SRC_DIR)utils.c\ + $(SRC_DIR)variable.c\ + $(SRC_DIR)vers.c + AP_OBJS = $(SRC_DIR)fics_addplayer.o MR_OBJS = $(SRC_DIR)makerank.o diff --git a/FICS/command.c b/FICS/command.c index fdf7cbb..569c27a 100644 --- a/FICS/command.c +++ b/FICS/command.c @@ -33,6 +33,9 @@ check_news() and rscan_news(). Markus Uhlin 24/11/25 Null checks + Markus Uhlin 25/03/09 Fixed double free() + Markus Uhlin 25/03/11 Fixed memleak + Markus Uhlin 25/03/16 Fixed use of 32-bit 'time_t' */ #include "stdinclude.h" @@ -129,7 +132,11 @@ parse_command(char *com_string, char **comm, char **parameters) PUBLIC int alias_lookup(char *tmp, alias_type *alias_list, int numalias) { - for (int i = 0; (alias_list[i].comm_name && i < numalias); i++) { + if (numalias >= MAX_ALIASES) + return -1; + for (int i = 0; + (i < numalias && alias_list[i].comm_name != NULL); + i++) { if (!strcmp(tmp, alias_list[i].comm_name)) return i; } @@ -571,12 +578,18 @@ process_login(int p, char *loginname) if (!alphastring(loginname)) { pprintf(p, "\nSorry, names can only consist of lower " "and upper case letters. Try again.\n"); + rfree(loginnameii); + loginnameii = NULL; } else if (strlen(loginname) < 3) { pprintf(p, "\nA name should be at least three " "characters long! Try again.\n"); + rfree(loginnameii); + loginnameii = NULL; } else if (strlen(loginname) > 17) { pprintf(p, "\nSorry, names may be at most 17 " "characters long. Try again.\n"); + rfree(loginnameii); + loginnameii = NULL; } else if (in_list(p, L_BAN, loginnameii)) { pprintf(p, "\nPlayer \"%s\" is banned.\n", loginname); rfree(loginnameii); @@ -636,12 +649,12 @@ process_login(int p, char *loginname) parray[p].status = PLAYER_PASSWORD; turn_echo_off(parray[p].socket); rfree(loginnameii); + loginnameii = NULL; // XXX if (strcasecmp(loginname, parray[p].name)) { pprintf(p, "\nYou've got a bad name field in " "your playerfile -- please report this to " "an admin!\n"); - rfree(loginnameii); return COM_LOGOUT; } @@ -652,7 +665,6 @@ process_login(int p, char *loginname) pprintf(p, "Your handle is missing!"); pprintf(p, "Please log on as an unreg until " "an admin can correct this.\n"); - rfree(loginnameii); return COM_LOGOUT; } @@ -663,7 +675,6 @@ process_login(int p, char *loginname) pprintf(p, "Your FullName is missing!"); pprintf(p, "Please log on as an unreg until " "an admin can correct this.\n"); - rfree(loginnameii); return COM_LOGOUT; } @@ -674,7 +685,6 @@ process_login(int p, char *loginname) pprintf(p, "Your Email address is missing\n"); pprintf(p, "Please log on as an unreg until " "an admin can correct this.\n"); - rfree(loginnameii); return COM_LOGOUT; } } @@ -704,7 +714,7 @@ boot_out(int p, int p1) } PUBLIC void -rscan_news(FILE *fp, int p, int lc) +rscan_news(FILE *fp, int p, time_t lc) { char *junkp = NULL; char count[10] = { '\0' }; @@ -1214,10 +1224,10 @@ PUBLIC int process_heartbeat(int *fd) { time_t now = time(NULL); - int time_since_last; - static int last_comfile = 0; - static int last_space = 0; - static int lastcalled = 0; + time_t time_since_last; + static time_t last_comfile = 0; + static time_t last_space = 0; + static time_t lastcalled = 0; if (lastcalled == 0) time_since_last = 0; diff --git a/FICS/comproc.c b/FICS/comproc.c index f1a95d4..a7a5ea1 100644 --- a/FICS/comproc.c +++ b/FICS/comproc.c @@ -36,6 +36,17 @@ Markus Uhlin 24/11/19 Improved com_news(). plogins: fscanf: added width specification + Markus Uhlin 25/03/08 Calc string length once + Markus Uhlin 25/03/11 Fixed possibly uninitialized + value 'rat' in who_terse(). + Markus Uhlin 25/03/16 Fixed use of 32-bit 'time_t'. + Markus Uhlin 25/03/16 Fixed untrusted array index. + Markus Uhlin 25/03/25 com_unalias: fixed overflowed + array index read/write. + Markus Uhlin 25/07/21 com_who: fixed multiplication + result converted to larger type. + Markus Uhlin 25/07/24 Fixed use of potentially + dangerous functions. */ #include "stdinclude.h" @@ -44,6 +55,7 @@ #include <sys/resource.h> #include <err.h> +#include <errno.h> #include "board.h" #include "command.h" @@ -380,11 +392,18 @@ com_stats_rating(char *hdr, statistics *stats, char *dest, const size_t dsize) stats->num); if (stats->whenbest) { + struct tm res = {0}; + snprintf(tmp, sizeof tmp, " %d", stats->best); strlcat(dest, tmp, dsize); - strftime(tmp, sizeof tmp, " (%d-%b-%y)", - localtime((time_t *) &stats->whenbest)); - strlcat(dest, tmp, dsize); + + errno = 0; + + if (localtime_r(&stats->whenbest, &res) != NULL) { + if (strftime(tmp, sizeof tmp, " (%d-%b-%y)", &res) != 0) + strlcat(dest, tmp, dsize); + } else + warn("%s: localtime_r", __func__); } if (strlcat(dest, "\n", dsize) >= dsize) { @@ -400,11 +419,10 @@ com_stats(int p, param_list param) (MAX_OBSERVE > MAX_SIMUL ? MAX_OBSERVE : MAX_SIMUL) char line[255] = { '\0' }; char tmp[255] = { '\0' }; - int g, i; + int g, i, t; int numbers[NUMBERS_SIZE]; int onTime; int p1, connected; - time_t t; if (param[0].type == TYPE_WORD) { if (!FindPlayer(p, param[0].val.word, &p1, &connected)) @@ -423,9 +441,11 @@ com_stats(int p, param_list param) snprintf(tmp, sizeof tmp, " Idle: %s\n", hms_desc(player_idle(p1))); } else { - if ((t = player_lastdisconnect(p1))) { + time_t last; + + if ((last = player_lastdisconnect(p1)) != 0) { snprintf(tmp, sizeof tmp, "(Last disconnected %s):\n", - strltime(&t)); + strltime(&last)); } else strlcpy(tmp, "(Never connected.)\n", sizeof tmp); } @@ -573,9 +593,14 @@ com_stats(int p, param_list param) if (connected && parray[p1].registered && (p == p1 || parray[p].adminLevel > 0)) { - char *timeToStr = ctime((time_t *) &parray[p1].timeOfReg); + char timeToStr[30] = { '\0' }; + + errno = 0; + + if (ctime_r(&parray[p1].timeOfReg, timeToStr) == NULL) + warn("%s: ctime_r", __func__); + timeToStr[strcspn(timeToStr, "\n")] = '\0'; - timeToStr[strlen(timeToStr) - 1] = '\0'; pprintf(p, "\n"); onTime = ((time(NULL) - parray[p1].logon_time) + @@ -737,9 +762,10 @@ plogins(int p, char *fname) FILE *fp = NULL; char ipstr[20] = { '\0' }; char loginName[MAX_LOGIN_NAME + 1] = { '\0' }; - int inout, registered; + int registered = 0; long int lval = 0; time_t tval = 0; + uint16_t inout = 0; if ((fp = fopen(fname, "r")) == NULL) { pprintf(p, "Sorry, no login information available.\n"); @@ -750,7 +776,7 @@ plogins(int p, char *fname) _Static_assert(19 < ARRAY_SIZE(loginName), "'loginName' too small"); while (!feof(fp)) { - if (fscanf(fp, "%d %19s %ld %d %19s\n", &inout, loginName, + if (fscanf(fp, "%hu %19s %ld %d %19s\n", &inout, loginName, &lval, ®istered, ipstr) != 5) { fprintf(stderr, "FICS: Error in login info format. " "%s\n", fname); @@ -760,8 +786,13 @@ plogins(int p, char *fname) tval = lval; - pprintf(p, "%s: %-17s %-6s", strltime(&tval), loginName, - inout_string[inout]); + if (inout >= ARRAY_SIZE(inout_string)) { + warnx("%s: %s: 'inout' too large (%u)", __func__, fname, + inout); + } else { + pprintf(p, "%s: %-17s %-6s", strltime(&tval), loginName, + inout_string[inout]); + } if (parray[p].adminLevel > 0) pprintf(p, " from %s\n", ipstr); @@ -842,6 +873,8 @@ who_terse(int p, int num, int *plist, int type) rat = parray[p1].s_stats.rating; else if (type == light_rat) rat = parray[p1].l_stats.rating; + else // Fallback to std... + rat = parray[p1].s_stats.rating; if (type == none) { strlcpy(ptmp, " ", sizeof ptmp); @@ -866,8 +899,10 @@ who_terse(int p, int num, int *plist, int type) } if (p == p1) { - psprintf_highlight(p, ptmp + strlen(ptmp), - sizeof ptmp - strlen(ptmp), "%s", parray[p1].name); + const size_t len = strlen(ptmp); + + psprintf_highlight(p, ptmp + len, + sizeof ptmp - len, "%s", parray[p1].name); } else { strlcat(ptmp, parray[p1].name, sizeof ptmp); } @@ -929,9 +964,12 @@ who_verbose(int p, int num, int plist[]) p1WithAttrs[17] = '\0'; if (p == p1) { + size_t len; + strlcpy(tmp, " ", sizeof tmp); - psprintf_highlight(p, tmp + strlen(tmp), - sizeof tmp - strlen(tmp), "%-17s", p1WithAttrs); + len = strlen(tmp); + psprintf_highlight(p, tmp + len, sizeof tmp - len, + "%-17s", p1WithAttrs); } else { ret = snprintf(tmp, sizeof tmp, " %-17s", p1WithAttrs); @@ -1299,10 +1337,7 @@ com_who(int p, param_list param) sel_bits |= WHO_REGISTERED; break; case 'l': // Sort order - cmp_func = alpha_cmp; - sort_type = none; - break; - case 'A': // Sort order + case 'A': cmp_func = alpha_cmp; sort_type = none; break; @@ -1359,8 +1394,8 @@ com_who(int p, param_list param) count++; } - startpoint = floor((float) count * start_perc); - stoppoint = ceil((float) count * stop_perc) - 1; + startpoint = floorf((float) count * start_perc); + stoppoint = ceilf((float) count * stop_perc) - 1; num_who = 0; count = 0; @@ -1620,14 +1655,32 @@ com_unalias(int p, param_list param) pprintf(p, "You have no alias named '%s'.\n", param[0].val.word); } else { + bool removed = false; + const int sz = (int) ARRAY_SIZE(parray[0].alias_list); + rfree(parray[p].alias_list[al].comm_name); rfree(parray[p].alias_list[al].alias); + parray[p].alias_list[al].comm_name = NULL; + parray[p].alias_list[al].alias = NULL; + for (int i = al; i < parray[p].numAlias; i++) { + if (i >= sz || i + 1 >= sz) { + warnx("%s: overflowed array index read/write", + __func__); + break; + } + parray[p].alias_list[i].comm_name = parray[p].alias_list[i + 1].comm_name; parray[p].alias_list[i].alias = parray[p].alias_list[i + 1].alias; + removed = true; + } + + if (!removed) { + pprintf(p, "Remove error.\n"); + return COM_FAILED; } parray[p].numAlias--; @@ -1771,10 +1824,9 @@ FindAndShowFile(int p, param_list param, char *dir) { char *iwant, *filenames[1000]; int i; - static char nullify = '\0'; if (param[0].type == TYPE_NULL) { - iwant = &nullify; + iwant = NULL; } else { iwant = param[0].val.word; @@ -1788,8 +1840,9 @@ FindAndShowFile(int p, param_list param, char *dir) i = search_directory(dir, iwant, filenames, ARRAY_SIZE(filenames)); if (i == 0) { - pprintf(p, "No information available on \"%s\".\n", iwant); - } else if (i == 1 || !strcmp(*filenames, iwant)) { + pprintf(p, "No information available on \"%s\".\n", + (iwant ? iwant : "")); + } else if (i == 1 || !strcmp(*filenames, iwant ? iwant : "")) { if (psend_file(p, dir, *filenames)) { /* * We should never reach this unless the file @@ -1800,7 +1853,7 @@ FindAndShowFile(int p, param_list param, char *dir) "Thank you.\n"); } } else { - if (*iwant) + if (iwant && *iwant) pprintf(p, "Matches:\n"); display_directory(p, filenames, i); } @@ -1828,7 +1881,6 @@ com_mailsource(int p, param_list param) char fname[MAX_FILENAME_SIZE]; char subj[120]; int count; - static char nullify = '\0'; if (!parray[p].registered) { pprintf(p, "Only registered people can use the mailsource " @@ -1837,14 +1889,15 @@ com_mailsource(int p, param_list param) } if (param[0].type == TYPE_NULL) - iwant = &nullify; + iwant = NULL; else iwant = param[0].val.word; if ((count = search_directory(source_dir, iwant, buffer, ARRAY_SIZE(buffer))) == 0) { - pprintf(p, "Found no source file matching \"%s\".\n", iwant); - } else if ((count == 1) || !strcmp(iwant, *buffer)) { + pprintf(p, "Found no source file matching \"%s\".\n", + (iwant ? iwant : "")); + } else if ((count == 1) || !strcmp(iwant ? iwant : "", *buffer)) { snprintf(subj, sizeof subj, "FICS source file from server " "%s: %s", fics_hostname, @@ -1860,7 +1913,7 @@ com_mailsource(int p, param_list param) } else { pprintf(p, "Found %d source files matching that:\n", count); - if (*iwant) { + if (iwant && *iwant) { display_directory(p, buffer, count); } else { // this junk is to get *.c *.h char *s; @@ -1891,7 +1944,6 @@ com_mailhelp(int p, param_list param) char subj[120]; int count; int lang = parray[p].language; - static char nullify = '\0'; if (!parray[p].registered) { pprintf(p, "Only registered people can use the mailhelp " @@ -1900,7 +1952,7 @@ com_mailhelp(int p, param_list param) } if (param[0].type == TYPE_NULL) - iwant = &nullify; + iwant = NULL; else iwant = param[0].val.word; @@ -1921,8 +1973,9 @@ com_mailhelp(int p, param_list param) } if (count == 0) { - pprintf(p, "Found no help file matching \"%s\".\n", iwant); - } else if (count == 1 || !strcmp(*buffer, iwant)) { + pprintf(p, "Found no help file matching \"%s\".\n", + (iwant ? iwant : "")); + } else if (count == 1 || !strcmp(*buffer, iwant ? iwant : "")) { snprintf(subj, sizeof subj, "FICS help file from server %s: %s", fics_hostname, *buffer); diff --git a/FICS/comproc.h b/FICS/comproc.h index bee30b9..30ef3ff 100644 --- a/FICS/comproc.h +++ b/FICS/comproc.h @@ -79,7 +79,7 @@ extern int com_uptime(int, param_list); extern int com_uscf(int, param_list); extern int com_who(int, param_list); -extern void rscan_news(FILE *, int, int); +extern void rscan_news(FILE *, int, time_t); extern void rscan_news2(FILE *, int, int); #endif /* _COMPROC_H */ diff --git a/FICS/fics_addplayer.c b/FICS/fics_addplayer.c index 80be9fe..ad6c43f 100644 --- a/FICS/fics_addplayer.c +++ b/FICS/fics_addplayer.c @@ -27,6 +27,8 @@ Markus Uhlin 24/04/01 Fixed empty hostname Markus Uhlin 24/04/04 Added usage of explicit_bzero() Markus Uhlin 24/05/25 Added command-line option 'a' + Markus Uhlin 25/03/23 Output restart notice if the + player is admin. */ #include "stdinclude.h" @@ -157,6 +159,11 @@ main(int argc, char *argv[]) funame, fname, email, password); printf("Admin: %s\n", (parray[p].adminLevel > 0 ? "Yes" : "No")); + if (parray[p].adminLevel > 0) { + printf("Player is admin. Please restart FICS in order for " + "the changes to take effect.\n"); + } + snprintf(text, sizeof text, "\nYour player account has been created.\n\n" "Login Name: %s\n" diff --git a/FICS/formula.c b/FICS/formula.c index ffbca20..dd5ff23 100644 --- a/FICS/formula.c +++ b/FICS/formula.c @@ -574,10 +574,11 @@ ChooseClauses(player *who, char *formula) return ret; for (i = 0; formula[i] != '\0' && formula[i] != '#'; i++) { - if (formula[i] != 'f' || (i > 0 && isalnum(formula[i - 1])) || - !isdigit(formula[i + 1])) + if ((i > 0 && isalnum(formula[i - 1])) || formula[i] != 'f' || + !isdigit(formula[i + 1]) || + sscanf(&formula[i], "f%d", &which) != 1) continue; - sscanf(&formula[i], "f%d", &which); + ret |= (1 << (which - 1)); } diff --git a/FICS/gamedb.c b/FICS/gamedb.c index 572abf5..2656447 100644 --- a/FICS/gamedb.c +++ b/FICS/gamedb.c @@ -36,6 +36,13 @@ Markus Uhlin 24/11/25 Null checks Markus Uhlin 24/12/02 Fixed bugs and ignored function return values. + Markus Uhlin 25/03/18 Fixed unchecked return values + Markus Uhlin 25/03/25 ReadGameState: fixed truncated + stdio return value. + Markus Uhlin 25/04/01 Fixed call of risky function + Markus Uhlin 25/04/01 ReadV1GameFmt: guard num half + moves. + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" @@ -43,6 +50,7 @@ #include <err.h> #include <errno.h> +#include <limits.h> #include "command.h" #include "config.h" @@ -974,8 +982,8 @@ WriteGameState(FILE *fp, game_state_t *gs) PRIVATE int ReadGameState(FILE *fp, game_state_t *gs, int version) { - char pieceChar; int i, j; + int pieceChar; int wkmoved, wqrmoved, wkrmoved, bkmoved, bqrmoved, bkrmoved; if (version == 0) { @@ -986,11 +994,12 @@ ReadGameState(FILE *fp, game_state_t *gs, int version) } } } else { - getc(fp); /* Skip past a newline. */ + (void) getc(fp); /* Skip past a newline. */ for (i = 0; i < 8; i++) { for (j = 0; j < 8; j++) { - pieceChar = getc(fp); + if ((pieceChar = getc(fp)) == EOF) + return -1; gs->board[i][j] = CharToPiece(pieceChar); } } @@ -1059,10 +1068,16 @@ got_attr_value(int g, char *attr, char *value, FILE *fp, char *file) } else if (!strcmp(attr, "type:")) { garray[g].type = atoi(value); } else if (!strcmp(attr, "halfmoves:")) { - garray[g].numHalfMoves = atoi(value); - - if (garray[g].numHalfMoves == 0) + if ((garray[g].numHalfMoves = atoi(value)) == 0) return 0; + else if (garray[g].numHalfMoves < 0 || + (size_t)garray[g].numHalfMoves > INT_MAX / sizeof(move_t)) { + warnx("%s: num half moves out-of-bounds (%d)", __func__, + garray[g].numHalfMoves); + return -1; + } else { + /* null */; + } garray[g].moveListSize = garray[g].numHalfMoves; garray[g].moveList = reallocarray(NULL, sizeof(move_t), @@ -1267,8 +1282,10 @@ ReadV1GameFmt(game *g, FILE *fp, const char *file, int version) _Static_assert(17 < ARRAY_SIZE(g->black_name), "Unexpected array size"); ret[0] = fscanf(fp, "%17s %17s", g->white_name, g->black_name); - ret[1] = fscanf(fp, "%d %d", &g->white_rating, &g->black_rating); - ret[2] = fscanf(fp, "%d %d %d %d", + ret[1] = fscanf(fp, "%d %d", // NOLINT + &g->white_rating, + &g->black_rating); + ret[2] = fscanf(fp, "%d %d %d %d", // NOLINT &g->wInitTime, &g->wIncrement, &g->bInitTime, @@ -1311,10 +1328,16 @@ ReadV1GameFmt(game *g, FILE *fp, const char *file, int version) ret[0] = fscanf(fp, "%d %d %d %d", &g->private, &g->type, &g->rated, &g->clockStopped); - ret[1] = fscanf(fp, "%d", &g->numHalfMoves); + ret[1] = fscanf(fp, "%d", &g->numHalfMoves); // NOLINT if (ret[0] != 4 || ret[1] != 1) { warnx("%s: fscanf error: %s", __func__, file); return -1; + } else if (g->numHalfMoves < 0 || (size_t)g->numHalfMoves > + INT_MAX / sizeof(move_t)) { + warnx("%s: warning: num half moves out-of-bounds (%d)", + __func__, + g->numHalfMoves); + return -1; } if (ReadV1Moves(g, fp) != 0) { @@ -1625,10 +1648,9 @@ RemoveHistGame(char *file, int maxlines) char Opponent[MAX_LOGIN_NAME + 1] = { '\0' }; char line[MAX_LINE_SIZE] = { '\0' }; int count = 0; - long int When, oppWhen; + long int When = 0, oppWhen = 0; _Static_assert(20 < ARRAY_SIZE(Opponent), "Not within bounds"); - When = oppWhen = 0; if ((fp = fopen(file, "r")) == NULL) { return; @@ -1840,38 +1862,46 @@ write_g_out(int g, char *file, int maxlines, int isDraw, char *EndSymbol, * Find from_spot in journal list - return 0 if corrupted */ PUBLIC int -journal_get_info(int p, char from_spot, char *WhiteName, int *WhiteRating, - char *BlackName, int *BlackRating, char *type, int *t, int *i, char *eco, - char *ending, char *result, char *fname) +journal_get_info(struct JGI_context *ctx, const char *fname) { FILE *fp; char count; if ((fp = fopen(fname, "r")) == NULL) { fprintf(stderr, "Corrupt journal file! %s\n", fname); - pprintf(p, "The journal file is corrupt! See an admin.\n"); + pprintf(ctx->p, "The journal file is corrupt! See an admin.\n"); return 0; } while (!feof(fp)) { - if (fscanf(fp, "%c %s %d %s %d %s %d %d %s %s %s\n", + _Static_assert(ARRAY_SIZE(ctx->WhiteName) > 20, + "'WhiteName' too small"); + _Static_assert(ARRAY_SIZE(ctx->BlackName) > 20, + "'BlackName' too small"); + + _Static_assert(ARRAY_SIZE(ctx->type) > 99, "'type' too small"); + _Static_assert(ARRAY_SIZE(ctx->eco) > 99, "'eco' too small"); + _Static_assert(ARRAY_SIZE(ctx->ending) > 99, "'ending' too small"); + _Static_assert(ARRAY_SIZE(ctx->result) > 99, "'result' too small"); + + if (fscanf(fp, "%c %20s %d %20s %d %99s %d %d %99s %99s %99s\n", &count, - WhiteName, &(*WhiteRating), - BlackName, &(*BlackRating), - type, - &(*t), &(*i), - eco, - ending, - result) != 11) { + ctx->WhiteName, &ctx->WhiteRating, + ctx->BlackName, &ctx->BlackRating, + ctx->type, + &ctx->t, &ctx->i, + ctx->eco, + ctx->ending, + ctx->result) != 11) { fprintf(stderr, "FICS: Error in journal info format. " "%s\n", fname); - pprintf(p, "The journal file is corrupt! Error in " + pprintf(ctx->p, "The journal file is corrupt! Error in " "internal format.\n"); fclose(fp); return 0; } - if (tolower(count) == from_spot) { + if (tolower(count) == ctx->from_spot) { fclose(fp); return 1; } @@ -1922,7 +1952,7 @@ addjournalitem(int p, char count2, char *WhiteName2, int WhiteRating2, ending2, result2); fclose(fp2); - rename(fname2, fname); + xrename(__func__, fname2, fname); return; } else { _Static_assert(ARRAY_SIZE(WhiteName) > 19, @@ -1996,7 +2026,7 @@ addjournalitem(int p, char count2, char *WhiteName2, int WhiteRating2, fclose(fp); fclose(fp2); - rename(fname2, fname); + xrename(__func__, fname2, fname); } PUBLIC int diff --git a/FICS/gamedb.h b/FICS/gamedb.h index d123028..8dc84d3 100644 --- a/FICS/gamedb.h +++ b/FICS/gamedb.h @@ -27,9 +27,11 @@ #ifndef _GAMEDB_H #define _GAMEDB_H +#include <stdint.h> #include <time.h> #include "board.h" +#include "command.h" extern const char *bstr[7]; extern const char *rstr[2]; @@ -139,15 +141,30 @@ typedef struct _game { move_t *examMoveList; // Extra movelist for examine int examMoveListSize; - unsigned int startTime; // The relative time the game started - unsigned int lastMoveTime; // Last time a move was made - unsigned int lastDecTime; // Last time a players clock was + uint64_t startTime; // The relative time the game started + uint64_t lastMoveTime; // Last time a move was made + uint64_t lastDecTime; // Last time a players clock was // decremented int result; int winner; } game; +struct JGI_context { + int p; + char from_spot; + char WhiteName[MAX_LOGIN_NAME + 1]; + int WhiteRating; + char BlackName[MAX_LOGIN_NAME + 1]; + int BlackRating; + char type[100]; + int t; + int i; + char eco[100]; + char ending[100]; + char result[100]; +}; + extern game *garray; extern int g_num; @@ -171,8 +188,7 @@ extern int game_remove(int); extern int game_save(int); extern int game_zero(int); extern int got_attr_value(int, char *, char *, FILE *, char *); -extern int journal_get_info(int, char, char *, int *, char *, int *, - char *, int *, int *, char *, char *, char *, char *); +extern int journal_get_info(struct JGI_context *, const char *); extern int pgames(int, int, char *); extern int pjournal(int, int, char *); extern void MakeFENpos(int, char *, size_t); diff --git a/FICS/gameproc.c b/FICS/gameproc.c index b4a538e..cd6398e 100644 --- a/FICS/gameproc.c +++ b/FICS/gameproc.c @@ -30,6 +30,7 @@ Markus Uhlin 24/04/13 Added usage of msnprintf(), mstrlcpy() and mstrlcat(). Markus Uhlin 24/05/05 Added usage of reallocarray(). + Markus Uhlin 25/03/09 Calc string length once */ #include "stdinclude.h" @@ -848,18 +849,23 @@ CheckRepetition(int p, int g) char *pos; int flag1 = 1, flag2 = 1; int move_num; + size_t len[3]; if (garray[g].numHalfMoves < 8) // Can't have three repeats any quicker. return 0; + len[0] = strlen(pos1); + len[1] = strlen(pos2); + for (move_num = garray[g].game_state.lastIrreversable; move_num < garray[g].numHalfMoves - 1; move_num++) { pos = GetFENpos(g, move_num); + len[2] = strlen(pos); - if (strlen(pos1) == strlen(pos) && !strcmp(pos1, pos)) + if (len[0] == len[2] && !strcmp(pos1, pos)) flag1++; - if (strlen(pos2) == strlen(pos) && !strcmp(pos2, pos)) + if (len[1] == len[2] && !strcmp(pos2, pos)) flag2++; } @@ -1828,7 +1834,11 @@ com_goboard(int p, param_list param) } on = parray[p].simul_info.onBoard; - g = parray[p].simul_info.boards[on]; + + if ((g = parray[p].simul_info.boards[on]) < 0) { + pprintf(p, "Internal error! Unexpected negative value!\n"); + return COM_OK; + } if (p1 == garray[g].black) { pprintf(p, "You are already at that board!\n"); diff --git a/FICS/legal2.c b/FICS/legal2.c index 528814a..15c0c16 100644 --- a/FICS/legal2.c +++ b/FICS/legal2.c @@ -1,7 +1,7 @@ #include "legal2.h" const char legalNotice2[] = - "Copyright (c) 2023, 2024 Markus Uhlin <maxxe@rpblc.net>\n" + "Copyright (c) 2023 - 2025 Markus Uhlin <maxxe@rpblc.net>\n" "\n" "Permission to use, copy, modify, and distribute this software for any\n" "purpose with or without fee is hereby granted, provided that the above\n" diff --git a/FICS/lists.c b/FICS/lists.c index d9879d4..e3f2873 100644 --- a/FICS/lists.c +++ b/FICS/lists.c @@ -60,7 +60,14 @@ list_find(int p, enum ListWhich l) int personal; personal = ListArray[l].rights == P_PERSONAL; - starter = (personal ? &parray[p].lists : &firstGlobalList); + + if (personal) { + if (p < 0) + return NULL; + starter = &parray[p].lists; + } else { + starter = &firstGlobalList; + } for (tempList = *starter; tempList != NULL; tempList = tempList->next) { if (l == tempList->which) { diff --git a/FICS/makerank.c b/FICS/makerank.c index bdc4d3b..4458f31 100644 --- a/FICS/makerank.c +++ b/FICS/makerank.c @@ -110,21 +110,33 @@ GetPlayerInfo(char *fileName, ENTRY *e) "strlcpy() truncated\n", __func__); } } else if (!strcmp(field, "S_NUM:")) { - sscanf(line, "%*s %d", &(e->r[0].num)); + if (sscanf(line, "%*s %d", &(e->r[0].num)) != 1) + warnx("%s: S_NUM error", __func__); } else if (!strcmp(field, "B_NUM:")) { - sscanf(line, "%*s %d", &(e->r[1].num)); + if (sscanf(line, "%*s %d", &(e->r[1].num)) != 1) + warnx("%s: B_NUM error", __func__); } else if (!strcmp(field, "W_NUM:")) { - sscanf(line, "%*s %d", &(e->r[2].num)); + if (sscanf(line, "%*s %d", &(e->r[2].num)) != 1) + warnx("%s: W_NUM error", __func__); } else if (!strcmp(field, "L_NUM:")) { - sscanf(line, "%*s %d", &(e->r[3].num)); + if (sscanf(line, "%*s %d", &(e->r[3].num)) != 1) + warnx("%s: L_NUM error", __func__); } else if (!strcmp(field, "S_RATING:")) { - sscanf(line, "%*s %d", &(e->r[0].rating)); + if (sscanf(line, "%*s %d", + &(e->r[0].rating)) != 1) + warnx("%s: S_RATING error", __func__); } else if (!strcmp(field, "B_RATING:")) { - sscanf(line, "%*s %d", &(e->r[1].rating)); + if (sscanf(line, "%*s %d", + &(e->r[1].rating)) != 1) + warnx("%s: B_RATING error", __func__); } else if (!strcmp(field, "W_RATING:")) { - sscanf(line, "%*s %d", &(e->r[2].rating)); + if (sscanf(line, "%*s %d", + &(e->r[2].rating)) != 1) + warnx("%s: W_RATING error", __func__); } else if (!strcmp(field, "L_RATING:")) { - sscanf(line, "%*s %d", &(e->r[3].rating)); + if (sscanf(line, "%*s %d", + &(e->r[3].rating)) != 1) + warnx("%s: L_RATING error", __func__); } else if (!strcmp(field, "Network:")) { done = 1; } diff --git a/FICS/matchproc.c b/FICS/matchproc.c index c66c688..1b1eab8 100644 --- a/FICS/matchproc.c +++ b/FICS/matchproc.c @@ -31,6 +31,9 @@ Markus Uhlin 24/12/02 com_accept: check that the accept number is within bounds. + Markus Uhlin 25/03/12 Fixed negative array + index read in + accept_match(). */ #include "stdinclude.h" @@ -312,7 +315,13 @@ accept_match(int p, int p1) unobserveAll(p); unobserveAll(p1); - which = player_find_pendfrom(p, p1, PEND_MATCH); + if ((which = player_find_pendfrom(p, p1, PEND_MATCH)) < 0) { + pprintf(p, "%s: player_find_pendfrom: error\n", __func__); + pprintf(p1, "%s accepted your challenge but a fatal error " + "occurred\n", parray[p].name); + return COM_FAILED; + } + pend = &parray[p].p_from_list[which]; wt = pend->param1; winc = pend->param2; @@ -1070,7 +1079,10 @@ com_match(int p, param_list param) .wt = wt, }; - print_bughouse(p, p1, &ctx, colorstr); + if (ctx.white >= 0) + print_bughouse(p, p1, &ctx, colorstr); + else + warnx("%s: cannot print bughouse", __func__); } if (in_list(p, L_COMPUTER, parray[p].name)) { diff --git a/FICS/movecheck.c b/FICS/movecheck.c index a0c9d30..5f607de 100644 --- a/FICS/movecheck.c +++ b/FICS/movecheck.c @@ -25,6 +25,8 @@ Markus Uhlin 23/12/24 Fixed dead assignment Markus Uhlin 24/05/05 Refactored and reformatted all functions. + Markus Uhlin 25/03/21 Fixed out-of-bounds array access + in has_legal_move(). */ #include "stdinclude.h" @@ -967,8 +969,10 @@ has_legal_move(game_state_t *gs) &numpossible); break; } - if (numpossible >= 500) + if (numpossible >= 500) { fprintf(stderr, "FICS: Possible move overrun\n"); + return 0; + } for (i = 0; i < numpossible; i++) { if (legal_andcheck_move(gs, f, r, possiblef[i], possibler[i])) diff --git a/FICS/network.c b/FICS/network.c index 1b8f822..1e97e20 100644 --- a/FICS/network.c +++ b/FICS/network.c @@ -10,6 +10,7 @@ #include <arpa/telnet.h> #include <netinet/in.h> +#include <err.h> #include <errno.h> #include "common.h" @@ -266,7 +267,9 @@ net_send_string(int fd, char *str, int format) if ((which = findConnection(fd)) < 0) return -1; while (*str) { - for (i = 0; str[i] >= ' '; i++) { + const int upbound = (int)strlen(str); + + for (i = 0; i < upbound && str[i] >= ' '; i++) { /* null */; } @@ -311,6 +314,7 @@ net_send_string(int fd, char *str, int format) break; case '\033': con[which].outPos -= 3; + // XXX: fallthrough here? default: sendme(which, str, 1); } @@ -331,12 +335,11 @@ net_send_string(int fd, char *str, int format) PUBLIC int readline2(comstr_t *cs, int who) { - int howmany, state, fd, pending; - unsigned char *start, *s, *d; - - static unsigned char ayt[] = "[Responding to AYT: Yes, I'm here.]\n"; - static unsigned char will_sga[] = { IAC, WILL, TELOPT_SGA, '\0' }; - static unsigned char will_tm[] = { IAC, WILL, TELOPT_TM, '\0' }; + int howmany, state, fd, v_pending; + static const uint8_t ayt[] = "[Responding to AYT: Yes, I'm here.]\n"; + static const uint8_t will_sga[] = { IAC, WILL, TELOPT_SGA, '\0' }; + static const uint8_t will_tm[] = { IAC, WILL, TELOPT_TM, '\0' }; + unsigned char *start, *s, *d; state = con[who].state; @@ -347,11 +350,11 @@ readline2(comstr_t *cs, int who) } s = start = con[who].inBuf; - pending = con[who].numPending; + v_pending = con[who].numPending; fd = con[who].fd; - if ((howmany = recv(fd, start + pending, MAX_STRING_LENGTH - 1 - - pending, 0)) == 0) { // error: they've disconnected + if ((howmany = recv(fd, start + v_pending, MAX_STRING_LENGTH - 1 - + v_pending, 0)) == 0) { // error: they've disconnected return -1; } else if (howmany == -1) { if (errno != EWOULDBLOCK) { // some other error @@ -365,9 +368,9 @@ readline2(comstr_t *cs, int who) } if (con[who].processed) - s += pending; + s += v_pending; else - howmany += pending; + howmany += v_pending; d = s; for (; howmany-- > 0; s++) { @@ -402,7 +405,7 @@ readline2(comstr_t *cs, int who) state = 3; // this is cheesy // but we aren't using em } else if (*s == AYT) { - send(fd, (char *)ayt, strlen((char *)ayt), 0); + send(fd, (char *)ayt, sizeof ayt - 1, 0); state = 2; } else if (*s == EL) { // erase line d = start; @@ -435,11 +438,13 @@ readline2(comstr_t *cs, int who) break; case 4: // got IAC DO if (*s == TELOPT_TM) { - send(fd, (char *)will_tm, - strlen((char *)will_tm), 0); + if (send(fd, (char *)will_tm, + sizeof will_tm - 1, 0) == -1) + warn("%s: cannot send", __func__); } else if (*s == TELOPT_SGA) { - send(fd, (char *)will_sga, - strlen((char *)will_sga), 0); + if (send(fd, (char *)will_sga, + sizeof will_sga - 1, 0) == -1) + warn("%s: cannot send", __func__); } state = 2; break; @@ -467,9 +472,9 @@ readline2(comstr_t *cs, int who) } PUBLIC int -net_init(int port) +net_init(int p_port) { - int opt; + int opt, ret; struct linger lingeropt; struct sockaddr_in serv_addr; @@ -501,22 +506,30 @@ net_init(int port) memset(&serv_addr, 0, sizeof serv_addr); serv_addr.sin_family = AF_INET; serv_addr.sin_addr.s_addr = htonl(INADDR_ANY); - serv_addr.sin_port = htons(port); + serv_addr.sin_port = htons(p_port); /* * Attempt to allow rebinding to the port... */ opt = 1; - setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, (char *) &opt, sizeof opt); + ret = setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, (char *) &opt, + sizeof opt); + if (ret == -1) + warn("%s: SO_REUSEADDR", __func__); opt = 1; - setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, (char *) &opt, sizeof opt); + ret = setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, (char *) &opt, + sizeof opt); + if (ret == -1) + warn("%s: SO_KEEPALIVE", __func__); lingeropt.l_onoff = 0; lingeropt.l_linger = 0; - setsockopt(sockfd, SOL_SOCKET, SO_LINGER, (char *) &lingeropt, + ret = setsockopt(sockfd, SOL_SOCKET, SO_LINGER, (char *) &lingeropt, sizeof(lingeropt)); + if (ret == -1) + warn("%s: SO_LINGER", __func__); if (bind(sockfd, (struct sockaddr *) &serv_addr, sizeof serv_addr) < 0) { fprintf(stderr, "FICS: can't bind local address. errno=%d\n", @@ -553,15 +566,23 @@ net_close_connection(int fd) PUBLIC void turn_echo_on(int fd) { - static unsigned char wont_echo[] = { IAC, WONT, TELOPT_ECHO, '\0' }; - send(fd, (char *) wont_echo, strlen((char *) wont_echo), 0); + int ret; + static unsigned char wont_echo[] = {IAC, WONT, TELOPT_ECHO, '\0'}; + + ret = send(fd, (char *)wont_echo, sizeof wont_echo - 1, 0); + if (ret == -1) + warn("%s: cannot send", __func__); } PUBLIC void turn_echo_off(int fd) { - static unsigned char will_echo[] = { IAC, WILL, TELOPT_ECHO, '\0' }; - send(fd, (char *) will_echo, strlen((char *) will_echo), 0); + int ret; + static unsigned char will_echo[] = {IAC, WILL, TELOPT_ECHO, '\0'}; + + ret = send(fd, (char *)will_echo, sizeof will_echo - 1, 0); + if (ret == -1) + warn("%s: cannot send", __func__); } PUBLIC unsigned int diff --git a/FICS/obsproc.c b/FICS/obsproc.c index af8bf2a..2353c60 100644 --- a/FICS/obsproc.c +++ b/FICS/obsproc.c @@ -29,6 +29,10 @@ Markus Uhlin 24/07/07 Fixed unhandled return values of fscanf(). Markus Uhlin 24/12/02 Improved old_mail_moves() + Markus Uhlin 25/01/18 Fixed -Wshadow + Markus Uhlin 25/03/15 Fixed possible buffer overflow + in FindHistory2(). + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" @@ -87,19 +91,22 @@ GameNumFromParam(int p, int *p1, parameter *param) PRIVATE int gamesortfunc(const void *i, const void *j) { + const int x = *(int *)i; + const int y = *(int *)j; + /* * examine mode games moved to top of "games" output */ - return (GetRating(&parray[garray[*(int *)i].white], - garray[*(int *)i].type) + - GetRating(&parray[garray[*(int *)i].black], - garray[*(int *)i].type) - - (garray[*(int *)i].status == GAME_EXAMINE ? 10000 : 0) - - GetRating(&parray[garray[*(int *)j].white], - garray[*(int *)j].type) - - GetRating(&parray[garray[*(int *)j].black], - garray[*(int *)j].type) + - (garray[*(int *)j].status == GAME_EXAMINE ? 10000 : 0)); + return (GetRating(&parray[garray[x].white], + garray[x].type) + + GetRating(&parray[garray[x].black], + garray[x].type) - + (garray[x].status == GAME_EXAMINE ? 10000 : 0) - + GetRating(&parray[garray[y].white], + garray[y].type) - + GetRating(&parray[garray[y].black], + garray[y].type) + + (garray[y].status == GAME_EXAMINE ? 10000 : 0)); } PUBLIC int @@ -146,6 +153,9 @@ com_games(int p, param_list param) wp = garray[i].white; bp = garray[i].black; + UNUSED_VAR(wp); + UNUSED_VAR(bp); + if ((!selected) && s && strncasecmp(s, garray[i].white_name, slen) && @@ -374,9 +384,8 @@ com_allobservers(int p, param_list param) start = 0; end = g_num; } else if ((obgame >= g_num) || - ((obgame < g_num) && - ((garray[obgame].status != GAME_ACTIVE) && - (garray[obgame].status != GAME_EXAMINE)))) { + (garray[obgame].status != GAME_ACTIVE && + garray[obgame].status != GAME_EXAMINE)) { pprintf(p, "There is no such game.\n"); return COM_OK; } else { @@ -954,7 +963,7 @@ ExamineAdjourned(int p, int p1, int p2) } PRIVATE char * -FindHistory(int p, int p1, int game) +FindHistory(int p, int p1, int p_game) { FILE *fpHist; int index = 0; @@ -974,12 +983,17 @@ FindHistory(int p, int p1, int game) ret = fscanf(fpHist, "%d %*c %*d %*c %*d %*s %*s %*d %*d %*d " "%*d %*s %*s %ld", &index, &when); - if (ret != 2) - warn("%s: %s: corrupt", __func__, &fileName[0]); - } while (!feof(fpHist) && index != game); + if (ret != 2) { + warnx("%s: %s: corrupt", __func__, fileName); + fclose(fpHist); + return NULL; + } + } while (!feof(fpHist) && + !ferror(fpHist) && + index != p_game); - if (feof(fpHist)) { - pprintf(p, "There is no history game %d for %s.\n", game, + if (feof(fpHist) || ferror(fpHist)) { + pprintf(p, "There is no history game %d for %s.\n", p_game, parray[p1].name); fclose(fpHist); return NULL; @@ -993,9 +1007,10 @@ FindHistory(int p, int p1, int game) } PRIVATE char * -FindHistory2(int p, int p1, int game, char *End) +FindHistory2(int p, int p1, int p_game, char *End, const size_t End_size) { FILE *fpHist; + char fmt[80] = { '\0' }; int index = 0; long int when = 0; static char fileName[MAX_FILENAME_SIZE]; @@ -1008,17 +1023,21 @@ FindHistory2(int p, int p1, int game, char *End) return NULL; } - do { - int ret; + msnprintf(fmt, sizeof fmt, "%%d %%*c %%*d %%*c %%*d %%*s %%*s %%*d " + "%%*d %%*d %%*d %%*s %%%zus %%ld\n", (End_size - 1)); - ret = fscanf(fpHist, "%d %*c %*d %*c %*d %*s %*s %*d %*d %*d " - "%*d %*s %s %ld", &index, End, &when); - if (ret != 3) - warn("%s: %s: corrupt", __func__, &fileName[0]); - } while (!feof(fpHist) && index != game); + do { + if (fscanf(fpHist, fmt, &index, End, &when) != 3) { + warnx("%s: %s: corrupt", __func__, fileName); + fclose(fpHist); + return NULL; + } + } while (!feof(fpHist) && + !ferror(fpHist) && + index != p_game); - if (feof(fpHist)) { - pprintf(p, "There is no history game %d for %s.\n", game, + if (feof(fpHist) || ferror(fpHist)) { + pprintf(p, "There is no history game %d for %s.\n", p_game, parray[p1].name); fclose(fpHist); return NULL; @@ -1032,15 +1051,15 @@ FindHistory2(int p, int p1, int game, char *End) } PRIVATE void -ExamineHistory(int p, int p1, int game) +ExamineHistory(int p, int p1, int p_game) { FILE *fpGame; char *fileName; - if ((fileName = FindHistory(p, p1, game)) != NULL) { + if ((fileName = FindHistory(p, p1, p_game)) != NULL) { if ((fpGame = fopen(fileName, "r")) == NULL) { pprintf(p, "History game %d not available for %s.\n", - game, parray[p1].name); + p_game, parray[p1].name); } else { ExamineStored(fpGame, p, fileName); fclose(fpGame); @@ -1751,21 +1770,13 @@ com_journal(int p, param_list param) PRIVATE void jsave_journalentry(int p, char save_spot, int p1, char from_spot, char *to_file) { - FILE *Game; - char *name_from = parray[p1].login; - char *name_to = parray[p].login; - char BlackName[MAX_LOGIN_NAME + 1]; - char WhiteName[MAX_LOGIN_NAME + 1]; - char command[MAX_FILENAME_SIZE * 2 + 3]; - char eco[100]; - char ending[100]; - char fname[MAX_FILENAME_SIZE]; - char fname2[MAX_FILENAME_SIZE]; - char result[100]; - char type[100]; - int BlackRating; - int WhiteRating; - int i, t; + FILE *Game; + char *name_from = parray[p1].login; + char *name_to = parray[p].login; + char command[MAX_FILENAME_SIZE * 2 + 3]; + char fname[MAX_FILENAME_SIZE]; + char fname2[MAX_FILENAME_SIZE]; + struct JGI_context ctx; msnprintf(fname, sizeof fname, "%s/%c/%s.%c", journal_dir, name_from[0], name_from, from_spot); @@ -1795,14 +1806,34 @@ jsave_journalentry(int p, char save_spot, int p1, char from_spot, char *to_file) msnprintf(fname, sizeof fname, "%s/player_data/%c/%s.%s", stats_dir, name_to[0], name_to, STATS_JOURNAL); - if (!journal_get_info(p, from_spot, WhiteName, &WhiteRating, - BlackName, &BlackRating, type, &t, &i, eco, ending, result, fname)) + /* + * Init context + */ + ctx.p = p; + ctx.from_spot = from_spot; + ctx.WhiteRating = 0; + ctx.BlackRating = 0; + ctx.t = 0; + ctx.i = 0; + memset(ctx.WhiteName, 0, sizeof(ctx.WhiteName)); + memset(ctx.BlackName, 0, sizeof(ctx.BlackName)); + memset(ctx.type, 0, sizeof(ctx.type)); + memset(ctx.eco, 0, sizeof(ctx.eco)); + memset(ctx.ending, 0, sizeof(ctx.ending)); + memset(ctx.result, 0, sizeof(ctx.result)); + + if (!journal_get_info(&ctx, fname)) return; addjournalitem(p, toupper(save_spot), - WhiteName, WhiteRating, - BlackName, BlackRating, - type, t, i, eco, ending, result, to_file); + ctx.WhiteName, ctx.WhiteRating, + ctx.BlackName, ctx.BlackRating, + ctx.type, + ctx.t, ctx.i, + ctx.eco, + ctx.ending, + ctx.result, + to_file); pprintf(p, "Journal entry %s %c saved in slot %c in journal.\n", parray[p1].name, toupper(from_spot), toupper(save_spot)); } @@ -1814,14 +1845,15 @@ jsave_history(int p, char save_spot, int p1, int from, char *to_file) char *EndSymbol; char *HistoryFname; char *name_to = parray[p].login; - char End[100]; - char command[MAX_FILENAME_SIZE * 2 + 3]; + char End[100] = { '\0' }; + char command[MAX_FILENAME_SIZE * 2 + 3] = { '\0' }; char filename[MAX_FILENAME_SIZE + 1] = { '\0' }; // XXX - char jfname[MAX_FILENAME_SIZE]; + char jfname[MAX_FILENAME_SIZE] = { '\0' }; char type[4]; int g; - if ((HistoryFname = FindHistory2(p, p1, from, End)) != NULL) { + if ((HistoryFname = FindHistory2(p, p1, from, End, sizeof End)) != + NULL) { if ((Game = fopen(HistoryFname, "r")) == NULL) { pprintf(p, "History game %d not available for %s.\n", from, diff --git a/FICS/playerdb.c b/FICS/playerdb.c index f9e9410..c85ec0c 100644 --- a/FICS/playerdb.c +++ b/FICS/playerdb.c @@ -35,12 +35,24 @@ Markus Uhlin 24/11/24 Fixed incorrect format strings Markus Uhlin 24/12/02 Made many improvements Markus Uhlin 24/12/04 Added player number checks + Markus Uhlin 25/02/11 Calc string length once + Markus Uhlin 25/03/22 Fixed overflowed return value in + player_search(). + Markus Uhlin 25/03/23 Fixed overflowed array index + read/write. + Markus Uhlin 25/03/29 player_remove_request: + fixed overflowed array index + read/write. + Markus Uhlin 25/04/02 add_to_list: added an upper + limit for the list size. + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" #include "common.h" #include <err.h> +#include <errno.h> #include <stdint.h> #include "command.h" @@ -74,6 +86,21 @@ player_num_ok_chk(const int num) num < (int)ARRAY_SIZE(parray)); } +PUBLIC void +xrename(const char *fn, const char *name1, const char *name2) +{ + if (fn == NULL || name1 == NULL || name2 == NULL) { + errno = EINVAL; + warn("%s", __func__); + return; + } + + errno = 0; + + if (rename(name1, name2) != 0) + warn("%s: '%s' -> '%s'", fn, name1, name2); +} + PRIVATE int get_empty_slot(void) { @@ -385,10 +412,14 @@ add_to_list(FILE *fp, enum ListWhich lw, int *size, int p) #define SCAN_STR "%1023s" - if (*size <= 0) + if (*size <= 0 || *size > MAX_GLOBAL_LIST_SIZE) { +// warnx("%s: illegal list size (%d)", __func__, *size); return -2; + } + while ((*size)-- > 0 && fscanf(fp, SCAN_STR, buf) == 1) list_add(p, lw, buf); + return (*size <= 0 ? 0 : -1); } @@ -411,7 +442,7 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) /* * Name */ - if (fgets(tmp2, sizeof tmp2, fp) != NULL && + if (fgets(tmp2, sizeof tmp2, fp) != NULL && // NOLINT strcmp(tmp2, "NONE\n") != 0) { tmp2[strcspn(tmp2, "\n")] = '\0'; pp->name = xstrdup(tmp2); @@ -422,7 +453,7 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) /* * Full name */ - if (fgets(tmp2, sizeof tmp2, fp) != NULL && + if (fgets(tmp2, sizeof tmp2, fp) != NULL && // NOLINT strcmp(tmp2, "NONE\n") != 0) { tmp2[strcspn(tmp2, "\n")] = '\0'; pp->fullName = xstrdup(tmp2); @@ -433,7 +464,7 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) /* * Password */ - if (fgets(tmp2, sizeof tmp2, fp) != NULL && + if (fgets(tmp2, sizeof tmp2, fp) != NULL && // NOLINT strcmp(tmp2, "NONE\n") != 0) { tmp2[strcspn(tmp2, "\n")] = '\0'; pp->passwd = xstrdup(tmp2); @@ -444,7 +475,7 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) /* * Email */ - if (fgets(tmp2, sizeof tmp2, fp) != NULL && + if (fgets(tmp2, sizeof tmp2, fp) != NULL && // NOLINT strcmp(tmp2, "NONE\n") != 0) { tmp2[strcspn(tmp2, "\n")] = '\0'; pp->emailAddress = xstrdup(tmp2); @@ -452,7 +483,9 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) pp->emailAddress = NULL; } - if (fscanf(fp, "%d %d %d %d %d %d %jd %d %jd %d %d %d %d %d %d %jd %d %jd " + if (feof(fp) || + ferror(fp) || + fscanf(fp, "%d %d %d %d %d %d %jd %d %jd %d %d %d %d %d %d %jd %d %jd " "%d %d %d %d %d %d %jd %d %jd %d %d %d %d %d %d %jd %d %jd %d %d %d %d " "%d %d %jd %d %jd %u\n", &pp->s_stats.num, &pp->s_stats.win, &pp->s_stats.los, @@ -554,6 +587,23 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) pp->timeOfReg = array[0]; pp->totalTime = array[1]; + if (pp->num_plan >= MAX_PLAN) { + warnx("Player %s is corrupt\nToo many plans (%d)", + parray[p].name, + pp->num_plan); + return; + } else if (pp->num_formula >= MAX_FORMULA) { + warnx("Player %s is corrupt\nToo many formulas (%d)", + parray[p].name, + pp->num_formula); + return; + } else if (pp->numAlias >= MAX_ALIASES) { + warnx("Player %s is corrupt\nToo many aliases (%d)", + parray[p].name, + pp->numAlias); + return; + } + if (pp->num_plan > 0) { for (i = 0; i < pp->num_plan; i++) { if (fgets(tmp2, sizeof tmp2, fp) == NULL) { @@ -618,14 +668,13 @@ ReadV1PlayerFmt(int p, player *pp, FILE *fp, char *file, int version) return; } - if (!(len = strlen(tmp2))) { + if (!strlen(tmp2)) { // XXX fprintf(stderr, "FICS: Error bad alias in " "file %s\n", file); i--; pp->numAlias--; } else { tmp2[strcspn(tmp2, "\n")] = '\0'; - tmp = tmp2; tmp = eatword(tmp2); *tmp = '\0'; tmp++; @@ -810,9 +859,11 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) * num_plan */ - parray[p].num_plan = atoi(value); - - if (parray[p].num_plan > 0) { + if ((parray[p].num_plan = atoi(value)) >= MAX_PLAN) { + warnx("%s: %s: too many plans (%d)", __func__, file, + parray[p].num_plan); + return -1; + } else if (parray[p].num_plan > 0) { for (i = 0; i < parray[p].num_plan; i++) { if (fgets(tmp, sizeof tmp, fp) == NULL) { @@ -834,15 +885,19 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) xstrdup(tmp) : NULL); } } + } else { + /* null */; } } else if (!strcmp(attr, "num_formula:")) { /* * num_formula */ - parray[p].num_formula = atoi(value); - - if (parray[p].num_formula > 0) { + if ((parray[p].num_formula = atoi(value)) >= MAX_FORMULA) { + warnx("%s: %s: too many formulas (%d)", __func__, file, + parray[p].num_formula); + return -1; + } else if (parray[p].num_formula > 0) { for (i = 0; i < parray[p].num_formula; i++) { if (fgets(tmp, sizeof tmp, fp) == NULL) { warnx("%s: bad formula: feof %s", @@ -863,6 +918,8 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) xstrdup(tmp) : NULL); } } + } else { + /* null */; } } else if (!strcmp(attr, "formula:")) { /* @@ -875,9 +932,11 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) * num_alias */ - parray[p].numAlias = atoi(value); - - if (parray[p].numAlias > 0) { + if ((parray[p].numAlias = atoi(value)) >= MAX_ALIASES) { + warnx("%s: %s: too many aliases (%d)", __func__, file, + parray[p].numAlias); + return -1; + } else if (parray[p].numAlias > 0) { for (i = 0; i < parray[p].numAlias; i++) { if (fgets(tmp, sizeof tmp, fp) == NULL) { warnx("%s: bad alias: feof %s", @@ -885,7 +944,7 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) return -1; } - if (!(len = strlen(tmp))) { + if (!strlen(tmp)) { // XXX fprintf(stderr, "FICS: Error bad alias " "in file %s\n", file); i--; @@ -904,13 +963,21 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) xstrdup(tmp1); } } + } else { + /* null */; } } else if (!strcmp(attr, "num_censor:")) { /* * num_censor */ - i = atoi(value); + if ((i = atoi(value)) < 0) { + warnx("%s: num censor negative", __func__); + return -1; + } else if (i > MAX_CENSOR) { + warnx("%s: num censor too large", __func__); + return -1; + } while (i--) { if (fgets(tmp, sizeof tmp, fp) == NULL) { @@ -929,7 +996,13 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) } } } else if (!strcmp(attr, "num_notify:")) { - i = atoi(value); + if ((i = atoi(value)) < 0) { + warnx("%s: num notify negative", __func__); + return -1; + } else if (i > MAX_NOTIFY) { + warnx("%s: num notify too large", __func__); + return -1; + } while (i--) { if (fgets(tmp, sizeof tmp, fp) == NULL) { @@ -948,7 +1021,10 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) } } } else if (!strcmp(attr, "num_noplay:")) { - i = atoi(value); + if ((i = atoi(value)) < 0) { + warnx("%s: num noplay negative", __func__); + return -1; + } while (i--) { if (fgets(tmp, sizeof tmp, fp) == NULL) { @@ -967,7 +1043,10 @@ got_attr_value_player(int p, char *attr, char *value, FILE *fp, char *file) } } } else if (!strcmp(attr, "num_gnotify:")) { - i = atoi(value); + if ((i = atoi(value)) < 0) { + warnx("%s: num gnotify negative", __func__); + return -1; + } while (i--) { if (fgets(tmp, sizeof tmp, fp) == NULL) { @@ -1023,7 +1102,7 @@ player_read(int p, char *name) } if (line[0] == 'v') - sscanf(line, "%*c %d", &version); + (void)sscanf(line, "%*c %d", &version); if (version > 0) // Quick method: ReadV1PlayerFmt(p, &parray[p], fp, fname, version); else { // Do it the old SLOW way @@ -1103,7 +1182,7 @@ player_markdeleted(int p) parray[p].login[0], parray[p].login); snprintf(fname2, sizeof fname2, "%s/%c/%s.delete", player_dir, parray[p].login[0], parray[p].login); - rename(fname, fname2); + xrename(__func__, fname, fname2); if ((fp = fopen(fname2, "a")) != NULL) { // Touch the file fprintf(fp, "\n"); @@ -1282,10 +1361,13 @@ player_find_part_login(char *name) { int found = -1; int i; + size_t namelen; if ((i = player_find_bylogin(name)) >= 0) return i; + namelen = strlen(name); + for (i = 0; i < p_num; i++) { if (parray[i].status == PLAYER_EMPTY || parray[i].status == PLAYER_LOGIN || @@ -1295,7 +1377,7 @@ player_find_part_login(char *name) if (!parray[i].login) continue; - if (!strncmp(parray[i].login, name, strlen(name))) { + if (!strncmp(parray[i].login, name, namelen)) { if (found >= 0) /* Ambiguous */ return -2; found = i; @@ -1766,17 +1848,26 @@ player_new_pendto(int p) PUBLIC int player_remove_pendto(int p, int p1, int type) { - int w; + bool removed = false; + int w; if ((w = player_find_pendto(p, p1, type)) < 0) return -1; - for (; w < (parray[p].num_to - 1); w++) + for (; w < (parray[p].num_to - 1); w++) { + if (w + 1 >= (int)ARRAY_SIZE(parray[0].p_to_list)) { + warnx("%s: overflowed array index write", __func__); + break; + } + parray[p].p_to_list[w] = parray[p].p_to_list[w + 1]; + removed = true; + } - parray[p].num_to = (parray[p].num_to - 1); + UNUSED_VAR(removed); + parray[p].num_to -= 1; - return 0; + return (0); } PUBLIC int @@ -1820,17 +1911,26 @@ player_new_pendfrom(int p) PUBLIC int player_remove_pendfrom(int p, int p1, int type) { - int w; + bool removed = false; + int w; if ((w = player_find_pendfrom(p, p1, type)) < 0) return -1; - for (; w < (parray[p].num_from - 1); w++) + for (; w < (parray[p].num_from - 1); w++) { + if (w + 1 >= (int)ARRAY_SIZE(parray[0].p_from_list)) { + warnx("%s: overflowed array index write", __func__); + break; + } + parray[p].p_from_list[w] = parray[p].p_from_list[w + 1]; + removed = true; + } - parray[p].num_from = (parray[p].num_from - 1); + UNUSED_VAR(removed); + parray[p].num_from -= 1; - return 0; + return (0); } PUBLIC int @@ -1865,28 +1965,44 @@ player_add_request(int p, int p1, int type, int param) PUBLIC int player_remove_request(int p, int p1, int type) { - int to = 0, from = 0; + bool removed; + int to = 0, from = 0; + + while (to != -1 && (to = player_find_pendto(p, p1, type)) != -1) { + removed = false; - while (to != -1) { - if ((to = player_find_pendto(p, p1, type)) != -1) { - for (; to < parray[p].num_to - 1; to++) { - parray[p].p_to_list[to] = - parray[p].p_to_list[to + 1]; + for (; to < parray[p].num_to - 1; to++) { + if (to + 1 >= (int)ARRAY_SIZE(parray[0].p_to_list)) { + warnx("%s: overflowed array index read/write", + __func__); + break; } - parray[p].num_to = (parray[p].num_to - 1); + parray[p].p_to_list[to] = parray[p].p_to_list[to + 1]; + removed = true; } + + UNUSED_VAR(removed); + parray[p].num_to -= 1; } - while (from != -1) { - if ((from = player_find_pendfrom(p1, p, type)) != -1) { - for (; from < parray[p1].num_from - 1; from++) { - parray[p1].p_from_list[from] = - parray[p1].p_from_list[from + 1]; + while (from != -1 && (from = player_find_pendfrom(p1, p, type)) != -1) { + removed = false; + + for (; from < parray[p1].num_from - 1; from++) { + if (from + 1 >= (int)ARRAY_SIZE(parray[0].p_from_list)) { + warnx("%s: overflowed array index read/write", + __func__); + break; } - parray[p1].num_from = (parray[p1].num_from - 1); + parray[p1].p_from_list[from] = + parray[p1].p_from_list[from + 1]; + removed = true; } + + UNUSED_VAR(removed); + parray[p1].num_from -= 1; } if ((type == PEND_ALL || type == PEND_MATCH) && parray[p].partner >= 0) @@ -2254,7 +2370,6 @@ player_goto_next_board(int p) on = parray[p].simul_info.onBoard; start = on; - g = -1; do { on++; @@ -2283,7 +2398,6 @@ player_goto_prev_board(int p) on = parray[p].simul_info.onBoard; start = on; - g = -1; do { --on; @@ -2722,7 +2836,7 @@ player_show_messages(int p) PUBLIC int ShowMsgsBySender(int p, param_list param) { - int nFrom, nTo; + int nFrom = -1, nTo = -1; int p1, connected; textlist *Head; @@ -2735,8 +2849,6 @@ ShowMsgsBySender(int p, param_list param) return -1; /* no need to disconnect */ } - nFrom = nTo = -1; - if (p != p1) { if ((nTo = LoadMsgs(p1, p + 1, &Head)) <= 0) { pprintf(p, "%s has no messages from you.\n", @@ -2825,8 +2937,11 @@ player_search(int p, char *name) int p1, count; // Exact match with connected player? - if ((p1 = player_find_bylogin(name)) >= 0) + if ((p1 = player_find_bylogin(name)) >= 0) { + if (p1 + 1 >= (int)ARRAY_SIZE(parray)) + return 0; return (p1 + 1); + } // Exact match with registered player? snprintf(pdir, sizeof pdir, "%s/%c", player_dir, name[0]); @@ -2882,7 +2997,7 @@ player_kill(char *name) name); snprintf(fname2, sizeof fname2, "%s/%c/.rem.%s", player_dir, name[0], name); - rename(fname, fname2); + xrename(__func__, fname, fname2); RemHist(name); @@ -2890,25 +3005,25 @@ player_kill(char *name) stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.games", stats_dir, name[0], name); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.comments", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.comments", stats_dir, name[0], name); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.logons", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.logons", stats_dir, name[0], name); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.messages", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.messages", stats_dir, name[0], name); - rename(fname, fname2); + xrename(__func__, fname, fname2); return 0; } @@ -2923,31 +3038,31 @@ player_rename(char *name, char *newname) name); snprintf(fname2, sizeof fname2, "%s/%c/%s", player_dir, newname[0], newname); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.games", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/%s.games", stats_dir, newname[0], newname); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.comments", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/%s.comments", stats_dir, newname[0], newname); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.logons", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/%s.logons", stats_dir, newname[0], newname); - rename(fname, fname2); + xrename(__func__, fname, fname2); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.messages", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/%s.messages", stats_dir, newname[0], newname); - rename(fname, fname2); + xrename(__func__, fname, fname2); return 0; } @@ -2962,31 +3077,31 @@ player_raise(char *name) name[0], name); snprintf(fname2, sizeof fname2, "%s/%c/.rem.%s", player_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.games", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.games", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.comments", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.comments", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.logons", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.logons", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.messages", stats_dir, name[0], name); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.messages", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); return 0; } @@ -3001,31 +3116,31 @@ player_reincarn(char *name, char *newname) newname[0], newname); snprintf(fname2, sizeof fname2, "%s/%c/.rem.%s", player_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.games", stats_dir, newname[0], newname); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.games", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.comments", stats_dir, newname[0], newname); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.comments", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.logons", stats_dir, newname[0], newname); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.logons", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); snprintf(fname, sizeof fname, "%s/player_data/%c/%s.messages", stats_dir, newname[0], newname); snprintf(fname2, sizeof fname2, "%s/player_data/%c/.rem.%s.messages", stats_dir, name[0], name); - rename(fname2, fname); + xrename(__func__, fname2, fname); return 0; } @@ -3067,11 +3182,13 @@ player_add_comment(int p_by, int p_to, char *comment) PUBLIC int player_show_comments(int p, int p1) { - char fname[MAX_FILENAME_SIZE]; + char fname[MAX_FILENAME_SIZE] = { '\0' }; snprintf(fname, sizeof fname, "%s/player_data/%c/%s.%s", stats_dir, parray[p1].login[0], parray[p1].login, "comments"); - psend_file(p, NULL, fname); + + if (psend_file(p, NULL, fname) == -1) + warnx("%s: psend_file() error", __func__); return 0; } diff --git a/FICS/playerdb.h b/FICS/playerdb.h index dc5d6cc..1ccae61 100644 --- a/FICS/playerdb.h +++ b/FICS/playerdb.h @@ -204,6 +204,7 @@ extern player parray[PARRAY_SIZE]; extern int p_num; extern bool player_num_ok_chk(const int); +extern void xrename(const char *, const char *, const char *); extern int ClearMsgsBySender(int, param_list); extern int ClrMsgRange(int, int, int); diff --git a/FICS/ratings.c b/FICS/ratings.c index 53ee33a..e445c51 100644 --- a/FICS/ratings.c +++ b/FICS/ratings.c @@ -31,6 +31,8 @@ Markus Uhlin 24/11/27 Added sscanf() width spec and fixed ignored retvals. Markus Uhlin 24/11/28 Added null checks + Markus Uhlin 25/03/16 Fixed use of 32-bit 'time_t'. + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" @@ -38,6 +40,8 @@ #include <err.h> #include <errno.h> +#include <limits.h> +#include <stdint.h> #include "command.h" #include "comproc.h" @@ -345,49 +349,52 @@ load_ratings(void) return; } - for (int i = 0; i < MAXHIST; i++) { - int ret, errno_save; + for (int i = 0; i < MAXHIST && !feof(fp) && !ferror(fp); i++) { + int ret; sHist[i] = bHist[i] = wHist[i] = lHist[i] = 0; - errno = 0; ret = fscanf(fp, "%d %d %d %d", &sHist[i], &bHist[i], &wHist[i], &lHist[i]); - errno_save = errno; if (ret != 4) { - if (feof(fp) || ferror(fp)) - break; - errno = errno_save; - warn("%s: too few items assigned (iteration: %d)", - __func__, i); + warnx("%s: %s: too few items assigned (iteration: %d)", + __func__, fname, i); + fclose(fp); + return; } } + if (ferror(fp)) { + warnx("%s: %s: the error indicator is set", __func__, fname); + fclose(fp); + return; + } + fclose(fp); - if (Rs_count) { - Ratings_S_StdDev = sqrt(Rs_S / Rs_count); + if (Rs_count != 0) { + Ratings_S_StdDev = sqrt(Rs_S / Rs_count); // NOLINT Ratings_S_Average = (Rs_total / (double)Rs_count); } else { Ratings_S_StdDev = 0; Ratings_S_Average = 0; } - if (Rb_count) { - Ratings_B_StdDev = sqrt(Rb_S / Rb_count); + if (Rb_count != 0) { + Ratings_B_StdDev = sqrt(Rb_S / Rb_count); // NOLINT Ratings_B_Average = (Rb_total / (double)Rb_count); } else { Ratings_B_StdDev = 0; Ratings_B_Average = 0; } - if (Rw_count) { - Ratings_W_StdDev = sqrt(Rw_S / Rw_count); + if (Rw_count != 0) { + Ratings_W_StdDev = sqrt(Rw_S / Rw_count); // NOLINT Ratings_W_Average = (Rw_total / (double)Rw_count); } else { Ratings_W_StdDev = 0; Ratings_W_Average = 0; } - if (Rl_count) { - Ratings_L_StdDev = sqrt(Rl_S / Rl_count); + if (Rl_count != 0) { + Ratings_L_StdDev = sqrt(Rl_S / Rl_count); // NOLINT Ratings_L_Average = (Rl_total / (double)Rl_count); } else { Ratings_L_StdDev = 0; @@ -754,7 +761,7 @@ GE(int r, int rr, double ss, double *fss) } PRIVATE double -current_sterr(double s, int t) +current_sterr(double s, int64_t t) { if (t < 0) t = 0; // this shouldn't happen @@ -769,15 +776,16 @@ current_sterr(double s, int t) * ics.onenet.net, if not elsewhere. */ PUBLIC void -rating_sterr_delta(int p1, int p2, int type, int gtime, int result, +rating_sterr_delta(int p1, int p2, int type, time_t gtime, int result, int *deltarating, double *newsterr) { double E, fs2, denominator, GK, w; // Parts of fancy formulas double delta, sigma; // Results to return double s1, s2; - int t1, r1, t2, r2; // Initial sterrs and ratings + int r1, r2; // Initial sterrs and ratings statistics *p1_stats; statistics *p2_stats; + time_t t1, t2; if (type == TYPE_BLITZ) { p1_stats = &parray[p1].b_stats; @@ -866,12 +874,12 @@ PUBLIC int rating_update(int g) { double wSigma, bSigma; - int gtime; int inprogress = (g == parray[garray[g].black].game); int wDelta, bDelta; int wRes, bRes; statistics *b_stats; statistics *w_stats; + time_t gtime; /* * If this is adjudication of stored game - be quiet about @@ -1300,6 +1308,18 @@ com_statistics(int p, param_list param) return COM_OK; } +/* + * Return the difference of 'a - b' + */ +PUBLIC int +int_diff(const char *fn, const int a, const int b) +{ + if ((b > 0 && a < INT_MIN + b) || + (b < 0 && a > INT_MAX + b)) + errx(1, "%s: integer overflow (%d - %d)", fn, a, b); + return (a - b); +} + PUBLIC int com_fixrank(int p, param_list param) { @@ -1579,16 +1599,20 @@ PositionFilePtr(FILE *fp, int count, int *last, int *nTied, int showComp) return; rating = nGames = is_computer = 0; + errno = 0; rewind(fp); + if (errno) { + warn("%s: rewind", __func__); + return; + } for (int i = 1; i < count; i++) { do { _Static_assert(ARRAY_SIZE(login) > 19, "'login' too small"); - if (fgets(line, sizeof line, fp) == NULL || - feof(fp) || - ferror(fp)) + if (feof(fp) || ferror(fp) || + fgets(line, sizeof line, fp) == NULL) break; else if (sscanf(line, "%19s %d %d %d", login, &rating, &nGames, &is_computer) != 4) { @@ -1597,6 +1621,11 @@ PositionFilePtr(FILE *fp, int count, int *last, int *nTied, int showComp) } } while (!CountRankLine(showComp, login, nGames, is_computer)); + if (ferror(fp)) { + warnx("%s: the error indicator is set", __func__); + return; + } + if (rating != *last) { *nTied = 1; *last = rating; @@ -1615,7 +1644,7 @@ ShowRankEntry(int p, FILE *fp, int count, int comp, char *target, // XXX rating = 0; - findable = (count > 0 && !feof(fp)); + findable = (count > 0 && !feof(fp) && !ferror(fp)); nGames = 0; is_comp = 0; @@ -1812,9 +1841,9 @@ DisplayTargetRank(int p, char *target, int show, int showComp) numAbove = CountAbove(numToShow, blitzRank, stdRank, wildRank, show); - blitzCount = (blitzRank - numAbove); - stdCount = (stdRank - numAbove); - wildCount = (wildRank - numAbove); + blitzCount = int_diff(__func__, blitzRank, numAbove); + stdCount = int_diff(__func__, stdRank, numAbove); + wildCount = int_diff(__func__, wildRank, numAbove); ShowRankLines(p, fb, fs, fw, blitzCount, stdCount, wildCount, numToShow, showComp, show, target); diff --git a/FICS/ratings.h b/FICS/ratings.h index 50b4f56..7f2a521 100644 --- a/FICS/ratings.h +++ b/FICS/ratings.h @@ -26,6 +26,8 @@ #ifndef _RATINGS_H #define _RATINGS_H +#include "common.h" + #define STATS_VERSION 2 #define RESULT_WIN 0 @@ -49,6 +51,7 @@ typedef struct _rateStruct { int rating; } rateStruct; +__FICS_BEGIN_DECLS extern int Best(int, param_list, int); extern int DisplayRank(int, param_list, int); extern int DisplayRankedPlayers(int, int, int, int, int); @@ -61,6 +64,7 @@ extern int com_hbest(int, param_list); extern int com_hrank(int, param_list); extern int com_rank(int, param_list); extern int com_statistics(int, param_list); +extern int int_diff(const char *, const int, const int); extern int is_active(int); extern int rating_delta(int, int, int, int, int); extern int rating_update(int); @@ -70,7 +74,8 @@ extern void rating_add(int, int); extern void rating_init(void); extern void rating_recalc(void); extern void rating_remove(int, int); -extern void rating_sterr_delta(int, int, int, int, int, int *, double *); +extern void rating_sterr_delta(int, int, int, time_t, int, int *, double *); extern void save_ratings(void); +__FICS_END_DECLS #endif /* _RATINGS_H */ diff --git a/FICS/talkproc.c b/FICS/talkproc.c index cfcf6c7..601f43b 100644 --- a/FICS/talkproc.c +++ b/FICS/talkproc.c @@ -33,6 +33,9 @@ Markus Uhlin 24/01/04 Fixed pprintf() calls Markus Uhlin 24/04/14 Refactored and reformatted ALL functions. + Markus Uhlin 25/01/18 Fixed -Wshadow + Markus Uhlin 25/04/04 tell: fixed constant expression + result */ #include "stdinclude.h" @@ -295,8 +298,8 @@ tell(int p, int p1, char *msg, int why, int ch) } if (player_censored(p1, p) && parray[p].adminLevel == 0) { - if (why != TELL_KIBITZ || - why != TELL_WHISPER || + if (why != TELL_KIBITZ && + why != TELL_WHISPER && why != TELL_CHANNEL) { pprintf(p, "Player \"%s\" is censoring you.\n", parray[p1].name); @@ -968,10 +971,10 @@ com_qtell(int p, param_list param) return COM_OK; } - mstrlcpy(buffer1, ":\0", sizeof buffer1); - mstrlcpy(buffer2, ":\0", sizeof buffer2); - mstrlcpy(buffer3, ":\0", sizeof buffer3); - mstrlcpy(buffer4, ":\0", sizeof buffer4); + mstrlcpy(buffer1, ":", sizeof buffer1); + mstrlcpy(buffer2, ":", sizeof buffer2); + mstrlcpy(buffer3, ":", sizeof buffer3); + mstrlcpy(buffer4, ":", sizeof buffer4); msnprintf(tmp, sizeof tmp, "%s", param[1].val.string); @@ -1034,7 +1037,6 @@ com_qtell(int p, param_list param) pprintf(p, "*qtell %s 0*\n", parray[p1].name); } else { int ch = param[0].val.integer; - int p1; if (ch == 0) { pprintf(p, "*qtell %d 1*\n", param[0].val.integer); diff --git a/FICS/utils.c b/FICS/utils.c index ee7b676..cf12871 100644 --- a/FICS/utils.c +++ b/FICS/utils.c @@ -36,6 +36,12 @@ ignored fgets() retvals. Markus Uhlin 24/12/02 Fixed a possible array overrun in truncate_file(). + Markus Uhlin 25/03/09 truncate_file: + fixed null ptr dereference. + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. + Markus Uhlin 25/07/21 Replaced non-reentrant functions + with their corresponding thread + safe version. */ #include "stdinclude.h" @@ -273,10 +279,10 @@ pcommand(int p, char *comstr, ...) return retval; } -PUBLIC int +PUBLIC void pprintf(int p, const char *format, ...) { - char tmp[10 * MAX_LINE_SIZE]; + char tmp[10 * MAX_LINE_SIZE] = { '\0' }; int retval; va_list ap; @@ -284,8 +290,8 @@ pprintf(int p, const char *format, ...) retval = vsnprintf(tmp, sizeof tmp, format, ap); va_end(ap); + UNUSED_VAR(retval); net_send_string(parray[p].socket, tmp, 1); - return retval; } PRIVATE void @@ -391,7 +397,7 @@ pprintf_noformat(int p, char *format, ...) } PUBLIC int -psend_raw_file(int p, char *dir, char *file) +psend_raw_file(int p, const char *dir, const char *file) { FILE *fp; char fname[MAX_FILENAME_SIZE] = { '\0' }; @@ -406,9 +412,18 @@ psend_raw_file(int p, char *dir, char *file) if ((fp = fopen(fname, "r")) == NULL) return -1; - while ((num = fread(tmp, sizeof(char), MAX_LINE_SIZE - 1, fp)) > 0) { - tmp[num] = '\0'; - net_send_string(parray[p].socket, tmp, 1); + while (!feof(fp) && !ferror(fp)) { + if ((num = fread(tmp, sizeof(char), MAX_LINE_SIZE - 1, + fp)) > 0) { + tmp[num] = '\0'; + net_send_string(parray[p].socket, tmp, 1); + } + } + + if (ferror(fp)) { + warnx("%s: %s: the error indicator is set", __func__, fname); + fclose(fp); + return -1; } fclose(fp); @@ -416,7 +431,7 @@ psend_raw_file(int p, char *dir, char *file) } PUBLIC int -psend_file(int p, char *dir, char *file) +psend_file(int p, const char *dir, const char *file) { FILE *fp; char fname[MAX_FILENAME_SIZE] = { '\0' }; @@ -436,12 +451,19 @@ psend_file(int p, char *dir, char *file) if ((fp = fopen(fname, "r")) == NULL) return -1; - while (!feof(fp) && --lcount > 0) { - if (fgets(tmp, sizeof tmp, fp) != NULL && !feof(fp)) - net_send_string(parray[p].socket, tmp, 1); + while (--lcount > 0) { + if (fgets(tmp, sizeof tmp, fp) == NULL) + break; + net_send_string(parray[p].socket, tmp, 1); } if (!feof(fp)) { + if (ferror(fp)) { + warnx("%s: %s: the error indicator is set", __func__, + fname); + fclose(fp); + return -1; + } parray[p].last_file = xstrdup(fname); parray[p].last_file_byte = ftell(fp); pprintf(p, "Type [next] to see next page.\n"); @@ -497,18 +519,30 @@ pmore_file(int p) if ((fp = fopen(parray[p].last_file, "r")) == NULL) { pprintf(p, "File not found!\n"); return -1; + } else if (fseek(fp, parray[p].last_file_byte, SEEK_SET) == -1) { + pprintf(p, "Unable to set the file position indicator.\n"); + fclose(fp); + return -1; } - fseek(fp, parray[p].last_file_byte, SEEK_SET); - - while (!feof(fp) && --lcount > 0) { - if (fgets(tmp, sizeof tmp, fp) != NULL && !feof(fp)) - net_send_string(parray[p].socket, tmp, 1); + while (--lcount > 0) { + if (fgets(tmp, sizeof tmp, fp) == NULL) + break; + net_send_string(parray[p].socket, tmp, 1); } if (!feof(fp)) { - parray[p].last_file_byte = ftell(fp); - pprintf(p, "Type [next] to see next page.\n"); + if (ferror(fp)) { + warnx("%s: %s: the error indicator is set", __func__, + parray[p].last_file); + fclose(fp); + return -1; + } else if ((parray[p].last_file_byte = ftell(fp)) == -1) { + warn("%s: %s: ftell", __func__, parray[p].last_file); + fclose(fp); + return -1; + } else + pprintf(p, "Type [next] to see next page.\n"); } else { rfree(parray[p].last_file); parray[p].last_file = NULL; @@ -723,19 +757,31 @@ fix_time(char *old_time) } PUBLIC char * -strltime(time_t *clock) +strltime(time_t *p_clock) { - struct tm *stm = localtime(clock); + struct tm stm = {0}; - return strtime(stm); + errno = 0; + + if (localtime_r(p_clock, &stm) == NULL) { + warn("%s: localtime_r", __func__); + memset(&stm, 0, sizeof stm); + } + return strtime(&stm); } PUBLIC char * -strgtime(time_t *clock) +strgtime(time_t *p_clock) { - struct tm *stm = gmtime(clock); + struct tm stm = {0}; - return strtime(stm); + errno = 0; + + if (gmtime_r(p_clock, &stm) == NULL) { + warn("%s: gmtime_r", __func__); + memset(&stm, 0, sizeof stm); + } + return strtime(&stm); } /* @@ -762,7 +808,7 @@ tenth_secs(void) * 2024-11-23 maxxe: changed the return type to 'time_t' */ PUBLIC time_t -untenths(unsigned int tenths) +untenths(uint64_t tenths) { return (tenths / 10 + 331939277 + 0xffffffff / 10 + 1); } @@ -807,7 +853,10 @@ truncate_file(char *file, int lines) fclose(fp); if (trunc) { - fp = fopen(file, "w"); + if ((fp = fopen(file, "w")) == NULL) { + warn("%s: fopen", __func__); + return 1; + } for (i = 0; i < lines; i++) { fputs(tBuf[bptr], fp); @@ -955,10 +1004,11 @@ ratstrii(int rat, int reg) * Fill 't_buffer' with anything matching "want*" in file tree */ PRIVATE void -t_sft(char *want, struct t_tree *t) +t_sft(const char *want, struct t_tree *t) { if (t) { - int cmp = strncmp(want, t->name, strlen(want)); + const char *v_want = (want ? want : ""); + int cmp = strncmp(v_want, t->name, strlen(v_want)); if (cmp <= 0) // If 'want' <= this one, look left t_sft(want, t->left); @@ -1041,7 +1091,6 @@ PUBLIC int search_directory(char *dir, char *filter, char **buffer, int buffersize) { int cmp; - static char nullify = '\0'; static struct t_dirs *ramdirs = NULL; struct stat statbuf; struct t_dirs** i; @@ -1050,9 +1099,6 @@ search_directory(char *dir, char *filter, char **buffer, int buffersize) t_buffersize = buffersize; if (!stat(dir, &statbuf)) { - if (filter == NULL) // NULL becomes pointer to null string - filter = &nullify; - i = &ramdirs; while (*i) { // Find dir in dir tree diff --git a/FICS/utils.h b/FICS/utils.h index b5f77c6..8f53882 100644 --- a/FICS/utils.h +++ b/FICS/utils.h @@ -31,6 +31,7 @@ #ifndef _UTILS_H #define _UTILS_H +#include <stdint.h> #include <stdio.h> #include "common.h" /* PRINTFLIKE() */ @@ -89,22 +90,22 @@ extern int mail_string_to_address(char *, char *, char *); extern int mail_string_to_user(int, char *, char *); extern int pcommand(int, char *, ...) PRINTFLIKE(2); extern int pmore_file(int); -extern int pprintf(int, const char *, ...) PRINTFLIKE(2); +extern void pprintf(int, const char *, ...) PRINTFLIKE(2); extern int pprintf_highlight(int, char *, ...) PRINTFLIKE(2); extern int pprintf_noformat(int, char *, ...) PRINTFLIKE(2); extern int pprintf_prompt(int, char *, ...) PRINTFLIKE(2); extern int printablestring(char *); extern int psend_command(int, char *, char *); -extern int psend_file(int, char *, char *); +extern int psend_file(int, const char *, const char *); extern int psend_logoutfile(int, char *, char *); -extern int psend_raw_file(int, char *, char *); +extern int psend_raw_file(int, const char *, const char *); extern int psprintf_highlight(int, char *, size_t, char *, ...) PRINTFLIKE(4); extern int safechar(int); extern int safestring(char *); extern int search_directory(char *, char *, char **, int); extern int truncate_file(char *, int); -extern time_t untenths(unsigned int); +extern time_t untenths(uint64_t); extern unsigned int tenth_secs(void); //extern void pprintf_dohightlight(int); //extern void sprintf_dohightlight(int, char *); diff --git a/FICS/vers.c b/FICS/vers.c index 64b0aac..c6dd99c 100644 --- a/FICS/vers.c +++ b/FICS/vers.c @@ -1,5 +1,5 @@ #include "vers.h" const char SGS_VERS[] = ""; -const char VERS_NUM[] = "rpblc-1.4.3"; +const char VERS_NUM[] = "rpblc-1.4.5"; const char COMP_DATE[] = __DATE__; diff --git a/GNUmakefile b/GNUmakefile index f94b0a4..cf29782 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -14,7 +14,8 @@ include FICS/build.mk # common rules include common.mk -.PHONY: clean install-init install +.PHONY: clean install-init install tidy include $(TARGETS_DIR)clean.mk include $(TARGETS_DIR)install.mk +include $(TARGETS_DIR)tidy.mk @@ -1,5 +1,7 @@ # README # + + ## About ## This is a fork of FICS(Free Internet Chess Server) version 1.6.2 made @@ -12,6 +14,18 @@ The main goal of the fork is to modernize the codebase, improve the security and fix bugs. New features, for example, other chess variants will be added in a later stage. +### Public chess server ### + +[IRCNow](https://ircnow.org/) +provides a +[public chess server](https://wiki.ircnow.org/index.php?n=Chess.Chess) +for everyone to use! + +To connect to the server by using +[XBoard](https://www.gnu.org/software/xboard/), try: + + $ xboard -ics -icshost rpblc.net + ### IPv6 ### IPv6 connections are at the moment not supported. @@ -37,6 +51,19 @@ repository: $ git clone https://github.com/uhlin/fics.git $ cd fics +### Checkout ### + +If you want you can checkout a specific version. For example: + + $ git checkout 1.4.4 + +(This is a good idea since you most likely don't want to run a +development version.) + +To see all tags, type: + + $ git tag + Edit `FICS/config.h` with a text editor and save the file. $ emacs FICS/config.h @@ -53,6 +80,10 @@ running `make install`. Done! +**NOTE**: +Running `make install` multiple times is totally fine and does no harm +when a new version of FICS is available. + ### Make variables ### If you want you can customize the `FICS_HOME` and `PREFIX` make diff --git a/include/colors.h b/include/colors.h new file mode 100644 index 0000000..6902138 --- /dev/null +++ b/include/colors.h @@ -0,0 +1,81 @@ +/* Copyright (c) 2019 Markus Uhlin <markus.uhlin@icloud.com> + All rights reserved. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL + WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED + WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE + AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL + DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR + PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR + PERFORMANCE OF THIS SOFTWARE. */ + +#ifndef _COLORS_H_ +#define _COLORS_H_ + +/* + * Reset all attributes to their defaults + */ +#define NORMAL "\x1b[0m" + +#define UNDERLINE_ON "\x1b[4m" +#define UNDERLINE_OFF "\x1b[24m" + +#define BLINK_ON "\x1b[5m" +#define BLINK_OFF "\x1b[25m" + +/* + * Reverse video + */ +#define REVVID_ON "\x1b[7m" +#define REVVID_OFF "\x1b[27m" + +#define BLACK "\x1b[30m" +#define RED "\x1b[31m" +#define GREEN "\x1b[32m" +#define BROWN "\x1b[33m" +#define BLUE "\x1b[34m" +#define MAGENTA "\x1b[35m" +#define CYAN "\x1b[36m" +#define WHITE "\x1b[37m" + +#define BOLDBLACK "\x1b[1;30m" +#define BOLDRED "\x1b[1;31m" +#define BOLDGREEN "\x1b[1;32m" +#define BOLDBROWN "\x1b[1;33m" +#define BOLDBLUE "\x1b[1;34m" +#define BOLDMAGENTA "\x1b[1;35m" +#define BOLDCYAN "\x1b[1;36m" +#define BOLDWHITE "\x1b[1;37m" + +#define UNDERSCORE_ON "\x1b[38m" +#define UNDERSCORE_OFF "\x1b[39m" + +#define BGBLACK "\x1b[40m" +#define BGRED "\x1b[41m" +#define BGGREEN "\x1b[42m" +#define BGBROWN "\x1b[43m" +#define BGBLUE "\x1b[44m" +#define BGMAGENTA "\x1b[45m" +#define BGCYAN "\x1b[46m" +#define BGWHITE "\x1b[47m" +#define BGDEFAULT "\x1b[49m" + +#if defined(_WIN32) && defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING) +static void +VirtualTerminalProcessing(void) +{ + HANDLE output_handle = GetStdHandle(STD_OUTPUT_HANDLE); + DWORD modes = 0; + + GetConsoleMode(output_handle, &modes); + modes |= ENABLE_VIRTUAL_TERMINAL_PROCESSING; + SetConsoleMode(output_handle, modes); +} +#endif + +#endif diff --git a/maketargets/tidy.mk b/maketargets/tidy.mk new file mode 100644 index 0000000..e52abe2 --- /dev/null +++ b/maketargets/tidy.mk @@ -0,0 +1,9 @@ +# The 'tidy' target + +TIDY = clang-tidy +TIDYFLAGS = -checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet +FICS_CLANG_TIDYFLAGS ?= + +tidy: $(INCLUDE_DIR)ficspaths.h + $(TIDY) $(SRCS) $(TIDYFLAGS) $(FICS_CLANG_TIDYFLAGS) -- \ + -I $(INCLUDE_DIR) $(CPPFLAGS) @@ -7,6 +7,8 @@ PREFIX ?= /home/chess CC ?= cc CFLAGS = -O2 -Wall -g -pipe -std=c11 \ + -Wformat-security \ + -Wshadow \ -Wsign-compare \ -Wstrict-prototypes diff --git a/tests/fscanf1.c b/tests/fscanf1.c new file mode 100644 index 0000000..366ea89 --- /dev/null +++ b/tests/fscanf1.c @@ -0,0 +1,36 @@ +#include <err.h> +#include <stdio.h> + +#include "maxxes-utils.h" + +int +main(void) +{ + FILE *fp; + char End[100] = { '\0' }; + char fmt[80] = { '\0' }; + const size_t End_size = sizeof End; + int index = 0; + long int when = 0; + + if ((fp = fopen("txt/stats-games.txt", "r")) == NULL) + err(1, "fopen"); + + msnprintf(fmt, sizeof fmt, "%%d %%*c %%*d %%*c %%*d %%*s %%*s %%*d " + "%%*d %%*d %%*d %%*s %%%zus %%ld\n", (End_size - 1)); + puts(fmt); + + do { + if (fscanf(fp, fmt, &index, End, &when) != 3) + warnx("items assigned mismatch"); + else { + printf("index:\t%d\nEnd:\t%s\nwhen:\t%ld\n---\n", + index, + End, + when); + } + } while (!feof(fp) && !ferror(fp)); + + fclose(fp); + return 0; +} diff --git a/tests/txt/stats-games.txt b/tests/txt/stats-games.txt new file mode 100644 index 0000000..f87a46b --- /dev/null +++ b/tests/txt/stats-games.txt @@ -0,0 +1,10 @@ +17 - 1425 B 1921 jrmu br 1200 120 1200 120 A00 Mat 1720893733 +18 = 0 W 0 kiliro puu 1200 120 1200 120 A00 Agr 1721235741 +19 = 1449 B 1692 kiliro br 1200 120 1200 120 *** Agr 1721235780 +20 = 0 W 0 kiliro puu 1200 120 1200 120 A00 Agr 1721237855 +21 - 1443 B 1948 jrmu br 1200 120 1200 120 A00 Mat 1721241134 +22 - 1443 W 0 bobbel bu 1200 120 1200 120 A00 Mat 1721252601 +23 - 1443 W 0 maxxetwo bu 1200 120 1200 120 A00 Mat 1722731908 +24 - 1443 W 0 maxxetwo bu 1200 120 1200 120 A00 Mat 1723503792 +25 = 1443 B 0 maxxetwo bu 1200 120 1200 120 A00 Agr 1733280040 +26 - 1443 W 0 maxxetwo bu 1200 120 1200 120 A00 Mat 1733280686 |