From cdd4ccf16ff6ccc202d3422fcd0ec7ca9f3a8229 Mon Sep 17 00:00:00 2001 From: Klemens Nanni Date: Fri, 11 Jun 2021 12:30:58 +0000 Subject: [PATCH] Open save file once in uiLoad() and keep it open until uiSave() Opening the same file *path* twice is a TOCTOU, although not a critical one: worst case we load from one file and save to another - the impact depends on how and when catgirl is started the next anyway. More importantly, keeping the file handle open at runtime allows us to drop all filesystem related promises for `-s/save' on OpenBSD. uiLoad() now opens "r+", meaning "Open for reading and writing." up front so uiSave() can write to it. In the case of a nonexistent save file, it now opens with "w" meaning "Open for writing. The file is created if it does not exist.", i.e. the same write/create semantics as "w" except uiLoad() no longer truncates. existing files. uiSave() now truncates the save file to avoid appending in general. --- chat.c | 2 +- chat.h | 2 +- ui.c | 12 +++++------- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/chat.c b/chat.c index e01b511..e6cd270 100644 --- a/chat.c +++ b/chat.c @@ -83,7 +83,7 @@ struct Self self = { .color = Default }; static const char *save; static void exitSave(void) { - int error = uiSave(save); + int error = uiSave(); if (error) { warn("%s", save); _exit(EX_IOERR); diff --git a/chat.h b/chat.h index 327262b..8c99cf5 100644 --- a/chat.h +++ b/chat.h @@ -312,7 +312,7 @@ void uiFormat( uint id, enum Heat heat, const time_t *time, const char *format, ... ) __attribute__((format(printf, 4, 5))); void uiLoad(const char *name); -int uiSave(const char *name); +int uiSave(void); enum { BufferCap = 1024 }; struct Buffer; diff --git a/ui.c b/ui.c index d8cb3e5..82c4716 100644 --- a/ui.c +++ b/ui.c @@ -1128,11 +1128,12 @@ static int writeString(FILE *file, const char *str) { return (fwrite(str, strlen(str) + 1, 1, file) ? 0 : -1); } -int uiSave(const char *name) { - FILE *saveFile = dataOpen(name, "w"); - if (!saveFile) return -1; +static FILE *saveFile; +int uiSave(void) { int error = 0 + || ftruncate(fileno(saveFile), 0) + || fseek(saveFile, 0, SEEK_SET) || writeTime(saveFile, Signatures[7]) || writeTime(saveFile, self.pos); if (error) return error; @@ -1179,12 +1180,11 @@ static ssize_t readString(FILE *file, char **buf, size_t *cap) { } void uiLoad(const char *name) { - FILE *saveFile = dataOpen(name, "r"); + saveFile = dataOpen(name, "r+"); if (!saveFile) { if (errno != ENOENT) exit(EX_NOINPUT); saveFile = dataOpen(name, "w"); if (!saveFile) exit(EX_CANTCREAT); - fclose(saveFile); return; } @@ -1192,7 +1192,6 @@ void uiLoad(const char *name) { fread(&signature, sizeof(signature), 1, saveFile); if (ferror(saveFile)) err(EX_IOERR, "fread"); if (feof(saveFile)) { - fclose(saveFile); return; } size_t version = signatureVersion(signature); @@ -1225,5 +1224,4 @@ void uiLoad(const char *name) { urlLoad(saveFile, version); free(buf); - fclose(saveFile); }