From 53ae0b83ae3e8ea56b4ee799d17df341ca6e2655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20=C3=89LIE?= Date: Tue, 26 Dec 2023 11:28:00 +0100 Subject: [PATCH] ovsqlite: Check the length of added overview data At read-time (search), ovsqlite-server implements a limit of 100,000 bytes to detect corrupt overview data. This patch adds the same limit when adding overview data, so that we do not add overview data into the database that could not be read afterwards. Thanks to Jesse Rehmer for the bug report (a newsgroup couldn't be expired because of a spam with 111,497 bytes of overview data that had been added into the database but was considered as corrupt when expiring the newsgroup). close #293 --- doc/pod/news.pod | 9 +++++++++ storage/ovsqlite/ovsqlite-private.h | 6 ++++++ storage/ovsqlite/ovsqlite-server.c | 8 +++----- storage/ovsqlite/ovsqlite.c | 13 ++++++++----- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/doc/pod/news.pod b/doc/pod/news.pod index ab0562204..bcfe362ca 100644 --- a/doc/pod/news.pod +++ b/doc/pod/news.pod @@ -108,6 +108,15 @@ flag) which had not the expected native LF line termination. =item * +Fixed an issue preventing articles from expiring when using the ovsqlite +method, in a very rare case. When an article had more than 100,000 bytes +of overview data (for instance with a Subject header field of that length), +overview expiration was no longer done for newsgroups carrying this article. +Such articles, which most certainly are spams anyway, are no longer added to +the ovsqlite database. Thanks for Jesse Rehmer for the bug report. + +=item * + Fixed a database lock issue when running B on a running server with the I parameter in F set to a higher value than the default busy timeout of 30 seconds of B. Thanks to diff --git a/storage/ovsqlite/ovsqlite-private.h b/storage/ovsqlite/ovsqlite-private.h index c1e9687ea..22b36924e 100644 --- a/storage/ovsqlite/ovsqlite-private.h +++ b/storage/ovsqlite/ovsqlite-private.h @@ -26,6 +26,12 @@ typedef struct ovsqlite_port { # endif /* ! HAVE_UNIX_DOMAIN_SOCKETS */ +/* A single overview record must not exceed the client buffer size, so make + * sure that SEARCHSPACE is always slightly greater than MAX_OVDATA_SIZE. + * 0x20000 corresponds to a client buffer size of 131072 bytes. */ +# define SEARCHSPACE 0x20000 +# define MAX_OVDATA_SIZE 100000 + /* * This needs to stay in sync with the dispatch array * in ovsqlite-server.c or things will explode. diff --git a/storage/ovsqlite/ovsqlite-server.c b/storage/ovsqlite/ovsqlite-server.c index 4cbe07cbc..7932fff15 100644 --- a/storage/ovsqlite/ovsqlite-server.c +++ b/storage/ovsqlite/ovsqlite-server.c @@ -43,11 +43,6 @@ # define OVSQLITE_DB_FILE "ovsqlite.db" -/* A single overview record must not exceed the client buffer size, so if - * MAX_OVDATA_SIZE is increased, SEARCHSPACE must also be increased in - * ovsqlite.c. */ -# define MAX_OVDATA_SIZE 100000 - # ifdef HAVE_ZLIB # define USE_DICTIONARY 1 @@ -1239,6 +1234,9 @@ do_add_article(client_t *client) if (!finish_request(client)) fail(response_bad_request); + if (overview_len > MAX_OVDATA_SIZE) + fail(response_corrupted); + begin_transaction(); stmt = sql_main.lookup_groupinfo; diff --git a/storage/ovsqlite/ovsqlite.c b/storage/ovsqlite/ovsqlite.c index 789b64889..110bc0f08 100644 --- a/storage/ovsqlite/ovsqlite.c +++ b/storage/ovsqlite/ovsqlite.c @@ -27,11 +27,6 @@ # include "../ovinterface.h" -/* A single overview record must not exceed the client buffer size, so if - * SEARCHSPACE is decreased, MAX_OVDATA_SIZE must also be decreased in - * ovsqlite-server.c. */ -# define SEARCHSPACE 0x20000 - typedef struct handle_t { uint8_t buffer[SEARCHSPACE]; uint64_t low; @@ -430,11 +425,19 @@ ovsqlite_add(const char *group, ARTNUM artnum, TOKEN token, char *data, warn("ovsqlite: not connected to server"); return false; } + groupname_len = strlen(group); r_artnum = artnum; overview_len = len; r_arrived = arrived; r_expires = expires; + + if (overview_len > MAX_OVDATA_SIZE) { + warn("Too large overview data of %d bytes (most certainly spam)", + overview_len); + return false; + } + start_request(request_add_article); pack_now(request, &groupname_len, sizeof groupname_len); pack_now(request, group, groupname_len);