From 876e1589f845febb4d4a7a5d29e8c533ed68bea4 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 28 May 2018 15:36:15 +0100 Subject: [PATCH] Rewrite conf deserialisation using BinarySource. Like the corresponding rewrite of conf serialisation, this affects not just conf_deserialise itself but also the per-platform filename and fontspec deserialisers. --- conf.c | 84 +++++++++++------------------------------------ putty.h | 6 ++-- unix/gtkmain.c | 56 ++++++++++++++++--------------- unix/uxmisc.c | 20 +++-------- windows/window.c | 5 ++- windows/winmisc.c | 31 +++++------------ 6 files changed, 68 insertions(+), 134 deletions(-) diff --git a/conf.c b/conf.c index 89d6412b..118e5944 100644 --- a/conf.c +++ b/conf.c @@ -504,97 +504,53 @@ void conf_serialise(BinarySink *bs, Conf *conf) put_uint32(bs, 0xFFFFFFFFU); } -int conf_deserialise(Conf *conf, void *vdata, int maxsize) +int conf_deserialise(Conf *conf, BinarySource *src) { - unsigned char *data = (unsigned char *)vdata; - unsigned char *start = data; struct conf_entry *entry; unsigned primary; - int used; - unsigned char *zero; - while (maxsize >= 4) { - primary = GET_32BIT_MSB_FIRST(data); - data += 4, maxsize -= 4; + while (1) { + primary = get_uint32(src); + if (get_err(src)) + return FALSE; + if (primary == 0xFFFFFFFFU) + return TRUE; if (primary >= N_CONFIG_OPTIONS) - break; + return FALSE; entry = snew(struct conf_entry); entry->key.primary = primary; switch (subkeytypes[entry->key.primary]) { case TYPE_INT: - if (maxsize < 4) { - sfree(entry); - goto done; - } - entry->key.secondary.i = toint(GET_32BIT_MSB_FIRST(data)); - data += 4, maxsize -= 4; + entry->key.secondary.i = toint(get_uint32(src)); break; case TYPE_STR: - zero = memchr(data, 0, maxsize); - if (!zero) { - sfree(entry); - goto done; - } - entry->key.secondary.s = dupstr((char *)data); - maxsize -= (zero + 1 - data); - data = zero + 1; + entry->key.secondary.s = dupstr(get_asciz(src)); break; } switch (valuetypes[entry->key.primary]) { case TYPE_INT: - if (maxsize < 4) { - if (subkeytypes[entry->key.primary] == TYPE_STR) - sfree(entry->key.secondary.s); - sfree(entry); - goto done; - } - entry->value.u.intval = toint(GET_32BIT_MSB_FIRST(data)); - data += 4, maxsize -= 4; + entry->value.u.intval = toint(get_uint32(src)); break; case TYPE_STR: - zero = memchr(data, 0, maxsize); - if (!zero) { - if (subkeytypes[entry->key.primary] == TYPE_STR) - sfree(entry->key.secondary.s); - sfree(entry); - goto done; - } - entry->value.u.stringval = dupstr((char *)data); - maxsize -= (zero + 1 - data); - data = zero + 1; + entry->value.u.stringval = dupstr(get_asciz(src)); break; case TYPE_FILENAME: - entry->value.u.fileval = - filename_deserialise(data, maxsize, &used); - if (!entry->value.u.fileval) { - if (subkeytypes[entry->key.primary] == TYPE_STR) - sfree(entry->key.secondary.s); - sfree(entry); - goto done; - } - data += used; - maxsize -= used; + entry->value.u.fileval = filename_deserialise(src); break; case TYPE_FONT: - entry->value.u.fontval = - fontspec_deserialise(data, maxsize, &used); - if (!entry->value.u.fontval) { - if (subkeytypes[entry->key.primary] == TYPE_STR) - sfree(entry->key.secondary.s); - sfree(entry); - goto done; - } - data += used; - maxsize -= used; + entry->value.u.fontval = fontspec_deserialise(src); break; } + + if (get_err(src)) { + free_entry(entry); + return FALSE; + } + conf_insert(conf, entry); } - - done: - return (int)(data - start); } diff --git a/putty.h b/putty.h index d7301509..fb9a2d55 100644 --- a/putty.h +++ b/putty.h @@ -1015,7 +1015,7 @@ void conf_set_filename(Conf *conf, int key, const Filename *val); void conf_set_fontspec(Conf *conf, int key, const FontSpec *val); /* Serialisation functions for Duplicate Session */ void conf_serialise(BinarySink *bs, Conf *conf); -int conf_deserialise(Conf *conf, void *data, int maxsize);/*returns size used*/ +int conf_deserialise(Conf *conf, BinarySource *src);/*returns true on success*/ /* * Functions to copy, free, serialise and deserialise FontSpecs. @@ -1029,7 +1029,7 @@ int conf_deserialise(Conf *conf, void *data, int maxsize);/*returns size used*/ FontSpec *fontspec_copy(const FontSpec *f); void fontspec_free(FontSpec *f); void fontspec_serialise(BinarySink *bs, FontSpec *f); -FontSpec *fontspec_deserialise(void *data, int maxsize, int *used); +FontSpec *fontspec_deserialise(BinarySource *src); /* * Exports from noise.c. @@ -1456,7 +1456,7 @@ int filename_is_null(const Filename *fn); Filename *filename_copy(const Filename *fn); void filename_free(Filename *fn); void filename_serialise(BinarySink *bs, const Filename *f); -Filename *filename_deserialise(void *data, int maxsize, int *used); +Filename *filename_deserialise(BinarySource *src); char *get_username(void); /* return value needs freeing */ char *get_random_data(int bytes, const char *device); /* used in cmdgen.c */ char filename_char_sanitise(char c); /* rewrite special pathname chars */ diff --git a/unix/gtkmain.c b/unix/gtkmain.c index ff6a15cf..906d029a 100644 --- a/unix/gtkmain.c +++ b/unix/gtkmain.c @@ -201,8 +201,9 @@ void launch_saved_session(const char *str) int read_dupsession_data(Conf *conf, char *arg) { - int fd, i, ret, size, size_used; + int fd, i, ret, size; char *data; + BinarySource src[1]; if (sscanf(arg, "---[%d,%d]", &fd, &size) != 2) { fprintf(stderr, "%s: malformed magic argument `%s'\n", appname, arg); @@ -222,35 +223,36 @@ int read_dupsession_data(Conf *conf, char *arg) exit(1); } - size_used = conf_deserialise(conf, data, size); - if (use_pty_argv && size > size_used) { - int n = 0; - i = size_used; - while (i < size) { - while (i < size && data[i]) i++; - if (i >= size) { - fprintf(stderr, "%s: malformed Duplicate Session data\n", - appname); - exit(1); - } - i++; - n++; - } - pty_argv = snewn(n+1, char *); - pty_argv[n] = NULL; - n = 0; - i = size_used; - while (i < size) { - char *p = data + i; - while (i < size && data[i]) i++; - assert(i < size); - i++; - pty_argv[n++] = dupstr(p); - } + BinarySource_BARE_INIT(src, data, size); + if (!conf_deserialise(conf, src)) { + fprintf(stderr, "%s: malformed Duplicate Session data\n", appname); + exit(1); + } + if (use_pty_argv) { + int pty_argc = 0; + size_t argv_startpos = src->pos; + + while (get_asciz(src), !get_err(src)) + pty_argc++; + + src->err = BSE_NO_ERROR; + + if (pty_argc > 0) { + src->pos = argv_startpos; + + pty_argv = snewn(pty_argc + 1, char *); + pty_argv[pty_argc] = NULL; + for (i = 0; i < pty_argc; i++) + pty_argv[i] = dupstr(get_asciz(src)); + } + } + + if (get_err(src) || get_avail(src) > 0) { + fprintf(stderr, "%s: malformed Duplicate Session data\n", appname); + exit(1); } sfree(data); - return 0; } diff --git a/unix/uxmisc.c b/unix/uxmisc.c index e96829c4..c478856b 100644 --- a/unix/uxmisc.c +++ b/unix/uxmisc.c @@ -78,16 +78,9 @@ void filename_serialise(BinarySink *bs, const Filename *f) { put_asciz(bs, f->path); } -Filename *filename_deserialise(void *vdata, int maxsize, int *used) +Filename *filename_deserialise(BinarySource *src) { - char *data = (char *)vdata; - char *end; - end = memchr(data, '\0', maxsize); - if (!end) - return NULL; - end++; - *used = end - data; - return filename_from_str(data); + return filename_from_str(get_asciz(src)); } char filename_char_sanitise(char c) @@ -274,14 +267,9 @@ void fontspec_serialise(BinarySink *bs, FontSpec *f) { put_asciz(bs, f->name); } -FontSpec *fontspec_deserialise(void *vdata, int maxsize, int *used) +FontSpec *fontspec_deserialise(BinarySource *src) { - char *data = (char *)vdata; - char *end = memchr(data, '\0', maxsize); - if (!end) - return NULL; - *used = end - data + 1; - return fontspec_new(data); + return fontspec_new(get_asciz(src)); } char *make_dir_and_check_ours(const char *dirname) diff --git a/windows/window.c b/windows/window.c index d6a67d42..88a251fe 100644 --- a/windows/window.c +++ b/windows/window.c @@ -480,7 +480,10 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) if (sscanf(p + 1, "%p:%u", &filemap, &cpsize) == 2 && (cp = MapViewOfFile(filemap, FILE_MAP_READ, 0, 0, cpsize)) != NULL) { - conf_deserialise(conf, cp, cpsize); + BinarySource src[1]; + BinarySource_BARE_INIT(src, cp, cpsize); + if (!conf_deserialise(conf, src)) + modalfatalbox("Serialised configuration data was invalid"); UnmapViewOfFile(cp); CloseHandle(filemap); } else if (!do_config()) { diff --git a/windows/winmisc.c b/windows/winmisc.c index c3ec8306..e65ba1c9 100644 --- a/windows/winmisc.c +++ b/windows/winmisc.c @@ -54,16 +54,9 @@ void filename_serialise(BinarySink *bs, const Filename *f) { put_asciz(bs, f->path); } -Filename *filename_deserialise(void *vdata, int maxsize, int *used) +Filename *filename_deserialise(BinarySource *src) { - char *data = (char *)vdata; - char *end; - end = memchr(data, '\0', maxsize); - if (!end) - return NULL; - end++; - *used = end - data; - return filename_from_str(data); + return filename_from_str(get_asciz(src)); } char filename_char_sanitise(char c) @@ -561,21 +554,13 @@ void fontspec_serialise(BinarySink *bs, FontSpec *f) put_uint32(bs, f->height); put_uint32(bs, f->charset); } -FontSpec *fontspec_deserialise(void *vdata, int maxsize, int *used) +FontSpec *fontspec_deserialise(BinarySource *src) { - char *data = (char *)vdata; - char *end; - if (maxsize < 13) - return NULL; - end = memchr(data, '\0', maxsize-12); - if (!end) - return NULL; - end++; - *used = end - data + 12; - return fontspec_new(data, - GET_32BIT_MSB_FIRST(end), - GET_32BIT_MSB_FIRST(end + 4), - GET_32BIT_MSB_FIRST(end + 8)); + const char *name = get_asciz(src); + unsigned isbold = get_uint32(src); + unsigned height = get_uint32(src); + unsigned charset = get_uint32(src); + return fontspec_new(name, isbold, height, charset); } int open_for_write_would_lose_data(const Filename *fn)