diff options
-rw-r--r-- | CHANGELOG.md | 37 | ||||
-rw-r--r-- | FICS/algcheck.c | 15 | ||||
-rw-r--r-- | FICS/board.c | 5 | ||||
-rw-r--r-- | FICS/build.mk | 34 | ||||
-rw-r--r-- | FICS/command.c | 6 | ||||
-rw-r--r-- | FICS/comproc.c | 53 | ||||
-rw-r--r-- | FICS/fics_addplayer.c | 7 | ||||
-rw-r--r-- | FICS/formula.c | 2 | ||||
-rw-r--r-- | FICS/gamedb.c | 84 | ||||
-rw-r--r-- | FICS/gamedb.h | 26 | ||||
-rw-r--r-- | FICS/gameproc.c | 6 | ||||
-rw-r--r-- | FICS/lists.c | 9 | ||||
-rw-r--r-- | FICS/makerank.c | 28 | ||||
-rw-r--r-- | FICS/matchproc.c | 5 | ||||
-rw-r--r-- | FICS/movecheck.c | 6 | ||||
-rw-r--r-- | FICS/network.c | 21 | ||||
-rw-r--r-- | FICS/obsproc.c | 110 | ||||
-rw-r--r-- | FICS/playerdb.c | 201 | ||||
-rw-r--r-- | FICS/ratings.c | 80 | ||||
-rw-r--r-- | FICS/ratings.h | 5 | ||||
-rw-r--r-- | FICS/talkproc.c | 6 | ||||
-rw-r--r-- | FICS/utils.c | 61 | ||||
-rw-r--r-- | FICS/utils.h | 6 | ||||
-rw-r--r-- | FICS/vers.c | 2 | ||||
-rw-r--r-- | GNUmakefile | 3 | ||||
-rw-r--r-- | README.md | 31 | ||||
-rw-r--r-- | include/colors.h | 81 | ||||
-rw-r--r-- | maketargets/tidy.mk | 9 |
28 files changed, 711 insertions, 228 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index bbcf774..8aab73c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,19 +3,32 @@ All notable changes to this fork of FICS version 1.6.2 will be documented in this file. -## [Unreleased] ## -- Changed the program to avoid calculating the same string multiple +## [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 double `free()` in `process_login()`. -- Fixed memory leak in `process_login()`. -- Fixed negative array index read in `accept_match()`. -- Fixed null pointer dereferences. -- Fixed possible buffer overflow in `FindHistory2()`. -- Fixed unchecked function return values. Multiple occurrences. -- Fixed uninitialized variables. -- Fixed untrusted array indices. -- Fixed use of 32-bit `time_t`. Y2K38 safety. Multiple occurrences. +- **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. 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 528bdf9..569c27a 100644 --- a/FICS/command.c +++ b/FICS/command.c @@ -132,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; } diff --git a/FICS/comproc.c b/FICS/comproc.c index 4f33d4c..2b5a04e 100644 --- a/FICS/comproc.c +++ b/FICS/comproc.c @@ -41,6 +41,8 @@ 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. */ #include "stdinclude.h" @@ -1318,10 +1320,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; @@ -1639,14 +1638,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--; @@ -1790,10 +1807,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; @@ -1807,8 +1823,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 @@ -1819,7 +1836,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); } @@ -1847,7 +1864,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 " @@ -1856,14 +1872,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, @@ -1879,7 +1896,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; @@ -1910,7 +1927,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 " @@ -1919,7 +1935,7 @@ com_mailhelp(int p, param_list param) } if (param[0].type == TYPE_NULL) - iwant = &nullify; + iwant = NULL; else iwant = param[0].val.word; @@ -1940,8 +1956,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/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 5f9fdb7..dd5ff23 100644 --- a/FICS/formula.c +++ b/FICS/formula.c @@ -574,7 +574,7 @@ 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])) || + if ((i > 0 && isalnum(formula[i - 1])) || formula[i] != 'f' || !isdigit(formula[i + 1]) || sscanf(&formula[i], "f%d", &which) != 1) continue; 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 eea5205..cd6398e 100644 --- a/FICS/gameproc.c +++ b/FICS/gameproc.c @@ -1834,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/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 fb13439..1b1eab8 100644 --- a/FICS/matchproc.c +++ b/FICS/matchproc.c @@ -1079,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 10e356d..1e97e20 100644 --- a/FICS/network.c +++ b/FICS/network.c @@ -267,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 */; } @@ -312,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); } @@ -471,7 +474,7 @@ readline2(comstr_t *cs, int who) PUBLIC int net_init(int p_port) { - int opt; + int opt, ret; struct linger lingeropt; struct sockaddr_in serv_addr; @@ -510,15 +513,23 @@ net_init(int p_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", diff --git a/FICS/obsproc.c b/FICS/obsproc.c index 283ecc3..2353c60 100644 --- a/FICS/obsproc.c +++ b/FICS/obsproc.c @@ -32,6 +32,7 @@ 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" @@ -90,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 @@ -149,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) && @@ -377,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 { @@ -977,11 +983,16 @@ FindHistory(int p, int p1, int p_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 != p_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)) { + if (feof(fpHist) || ferror(fpHist)) { pprintf(p, "There is no history game %d for %s.\n", p_game, parray[p1].name); fclose(fpHist); @@ -1016,11 +1027,16 @@ FindHistory2(int p, int p1, int p_game, char *End, const size_t End_size) "%%*d %%*d %%*d %%*s %%%zus %%ld\n", (End_size - 1)); do { - if (fscanf(fpHist, fmt, &index, End, &when) != 3) - warn("%s: %s: corrupt", __func__, &fileName[0]); - } while (!feof(fpHist) && index != p_game); + 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)) { + if (feof(fpHist) || ferror(fpHist)) { pprintf(p, "There is no history game %d for %s.\n", p_game, parray[p1].name); fclose(fpHist); @@ -1754,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); @@ -1798,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)); } diff --git a/FICS/playerdb.c b/FICS/playerdb.c index 7424edd..c85ec0c 100644 --- a/FICS/playerdb.c +++ b/FICS/playerdb.c @@ -36,6 +36,16 @@ 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" @@ -402,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); } @@ -428,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); @@ -439,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); @@ -450,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); @@ -461,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); @@ -469,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, @@ -571,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) { @@ -635,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++; @@ -827,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) { @@ -851,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", @@ -880,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:")) { /* @@ -892,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", @@ -902,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--; @@ -921,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) { @@ -946,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) { @@ -965,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) { @@ -984,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) { @@ -1040,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 @@ -1786,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 @@ -1840,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 @@ -1885,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) @@ -2274,7 +2370,6 @@ player_goto_next_board(int p) on = parray[p].simul_info.onBoard; start = on; - g = -1; do { on++; @@ -2303,7 +2398,6 @@ player_goto_prev_board(int p) on = parray[p].simul_info.onBoard; start = on; - g = -1; do { --on; @@ -2742,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; @@ -2755,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", @@ -2845,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]); @@ -3087,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/ratings.c b/FICS/ratings.c index b860260..e445c51 100644 --- a/FICS/ratings.c +++ b/FICS/ratings.c @@ -32,6 +32,7 @@ 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" @@ -39,6 +40,8 @@ #include <err.h> #include <errno.h> +#include <limits.h> +#include <stdint.h> #include "command.h" #include "comproc.h" @@ -346,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; @@ -755,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 @@ -776,9 +782,10 @@ rating_sterr_delta(int p1, int p2, int type, time_t gtime, int result, 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; @@ -1301,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) { @@ -1580,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) { @@ -1598,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; @@ -1616,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; @@ -1813,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 4cf47a8..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); @@ -72,5 +76,6 @@ extern void rating_recalc(void); extern void rating_remove(int, int); 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 3d16be1..601f43b 100644 --- a/FICS/talkproc.c +++ b/FICS/talkproc.c @@ -34,6 +34,8 @@ 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" @@ -296,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); diff --git a/FICS/utils.c b/FICS/utils.c index 1435e77..fd0a8e3 100644 --- a/FICS/utils.c +++ b/FICS/utils.c @@ -38,6 +38,7 @@ in truncate_file(). Markus Uhlin 25/03/09 truncate_file: fixed null ptr dereference. + Markus Uhlin 25/04/06 Fixed Clang Tidy warnings. */ #include "stdinclude.h" @@ -275,10 +276,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; @@ -286,8 +287,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 @@ -393,7 +394,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' }; @@ -408,9 +409,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); @@ -418,7 +428,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' }; @@ -445,6 +455,12 @@ psend_file(int p, char *dir, char *file) } 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"); @@ -506,14 +522,24 @@ pmore_file(int p) 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)) { - 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; @@ -963,10 +989,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); @@ -1049,7 +1076,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; @@ -1058,9 +1084,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 5342be2..8f53882 100644 --- a/FICS/utils.h +++ b/FICS/utils.h @@ -90,15 +90,15 @@ 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); diff --git a/FICS/vers.c b/FICS/vers.c index d556d4a..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.4"; +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) |