From 35d6474da608623482a95e89fae79420ec7eeb1b Mon Sep 17 00:00:00 2001 From: Sergey Poznyakoff Date: Tue, 16 Mar 2021 13:10:16 +0200 Subject: Verify if key/pair ends at a valid offset before attempting to read it. This fixes https://puszcza.gnu.org.ua/bugs/index.php?502. * src/findkey.c (gdbm_bucket_element_valid_p): Verify also if the key/data pair ends at a valid file offset. (_gdbm_read_entry): Set up the cache data only on success. * src/gdbmdefs.h (gdbm_bucket_element_valid_p): Remove declaration. (gdbm_file_info): New member 'file_size' keeps the recently retrieved value of the file size. It is invalidated wherever data is written to the database. * src/fullio.c (_gdbm_full_write,_gdbm_file_extend): Invalidate dbf->file_size. * src/gdbmopen.c (gdbm_fd_open): Initialize file_size to -1. (_gdbm_file_size): New function. * src/mmap.c (_gdbm_file_size): Remove. * src/proto.h (_gdbm_file_size): New proto. --- src/findkey.c | 39 +++++++++++++++++++++++++++++---------- src/fullio.c | 6 ++++++ src/gdbmdefs.h | 5 +++-- src/gdbmopen.c | 19 ++++++++++++++++++- src/mmap.c | 15 --------------- src/proto.h | 2 ++ 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/findkey.c b/src/findkey.c index 1d7b92d..ac777ed 100644 --- a/src/findkey.c +++ b/src/findkey.c @@ -21,11 +21,24 @@ #include "gdbmdefs.h" -int +/* Return true if OFF is a valid offset for GDBM_FILE */ +static inline int +gdbm_offset_ok (GDBM_FILE dbf, off_t off) +{ + off_t filesize; + + if (_gdbm_file_size (dbf, &filesize)) + return 0; + return off <= filesize; +} + +/* Return true if the element of hash table at index ELEM_LOC is a valid + hash element and represents a key/data pair that can be retrieved from + DBF. */ +static inline int gdbm_bucket_element_valid_p (GDBM_FILE dbf, int elem_loc) { - return - elem_loc < dbf->header->bucket_elems + return elem_loc < dbf->header->bucket_elems && dbf->bucket->h_table[elem_loc].hash_value != -1 && dbf->bucket->h_table[elem_loc].key_size >= 0 && off_t_sum_ok (dbf->bucket->h_table[elem_loc].data_pointer, @@ -33,7 +46,11 @@ gdbm_bucket_element_valid_p (GDBM_FILE dbf, int elem_loc) && dbf->bucket->h_table[elem_loc].data_size >= 0 && off_t_sum_ok (dbf->bucket->h_table[elem_loc].data_pointer + dbf->bucket->h_table[elem_loc].key_size, - dbf->bucket->h_table[elem_loc].data_size); + dbf->bucket->h_table[elem_loc].data_size) + && gdbm_offset_ok (dbf, + dbf->bucket->h_table[elem_loc].data_pointer + + dbf->bucket->h_table[elem_loc].key_size + + dbf->bucket->h_table[elem_loc].data_size); } /* Read the data found in bucket entry ELEM_LOC in file DBF and @@ -65,12 +82,8 @@ _gdbm_read_entry (GDBM_FILE dbf, int elem_loc) dsize = key_size + data_size; data_ca = &dbf->cache_entry->ca_data; - /* Set up the cache. */ - data_ca->key_size = key_size; - data_ca->data_size = data_size; - data_ca->elem_loc = elem_loc; - data_ca->hash_val = dbf->bucket->h_table[elem_loc].hash_value; - + /* Make sure data_ca has sufficient space to accomodate both + key and content. */ if (dsize <= data_ca->dsize) { if (data_ca->dsize == 0) @@ -122,6 +135,12 @@ _gdbm_read_entry (GDBM_FILE dbf, int elem_loc) _gdbm_fatal (dbf, gdbm_db_strerror (dbf)); return NULL; } + + /* Set up the cache. */ + data_ca->key_size = key_size; + data_ca->data_size = data_size; + data_ca->elem_loc = elem_loc; + data_ca->hash_val = dbf->bucket->h_table[elem_loc].hash_value; return data_ca->dptr; } diff --git a/src/fullio.c b/src/fullio.c index 77c3d27..a15b2fd 100644 --- a/src/fullio.c +++ b/src/fullio.c @@ -53,6 +53,9 @@ int _gdbm_full_write (GDBM_FILE dbf, void *buffer, size_t size) { char *ptr = buffer; + + /* Invalidate file_size */ + dbf->file_size = -1; while (size) { ssize_t wrbytes = gdbm_file_write (dbf, ptr, size); @@ -103,6 +106,9 @@ _gdbm_file_extend (GDBM_FILE dbf, off_t size) return -1; } + /* Invalidate file_size */ + dbf->file_size = -1; + while (size) { ssize_t n = write (dbf->desc, buf, diff --git a/src/gdbmdefs.h b/src/gdbmdefs.h index b1e53f9..bfa05c6 100644 --- a/src/gdbmdefs.h +++ b/src/gdbmdefs.h @@ -111,8 +111,6 @@ typedef struct int data_size; /* Size of associated data in the file. */ } bucket_element; -extern int gdbm_bucket_element_valid_p (GDBM_FILE dbf, int elem_loc); - /* A bucket is a small hash table. This one consists of a number of bucket elements plus some bookkeeping fields. The number of elements depends on the optimum blocksize for the storage device and on a @@ -273,6 +271,9 @@ struct gdbm_file_info unsigned bucket_changed :1; unsigned second_changed :1; + off_t file_size; /* Cached value of the current disk file size. + If -1, fstat will be used to retrieve it. */ + /* Mmap info */ size_t mapped_size_max;/* Max. allowed value for mapped_size */ void *mapped_region; /* Mapped region */ diff --git a/src/gdbmopen.c b/src/gdbmopen.c index cbb7195..c769643 100644 --- a/src/gdbmopen.c +++ b/src/gdbmopen.c @@ -286,7 +286,8 @@ gdbm_fd_open (int fd, const char *file_name, int block_size, /* Initialize cache */ dbf->cache_tree = _gdbm_cache_tree_alloc (); _gdbm_cache_init (dbf, DEFAULT_CACHESIZE); - + + dbf->file_size = -1; dbf->memory_mapping = FALSE; dbf->mapped_size_max = SIZE_T_MAX; dbf->mapped_region = NULL; @@ -719,3 +720,19 @@ gdbm_open (const char *file, int block_size, int flags, int mode, fatal_func); } +int +_gdbm_file_size (GDBM_FILE dbf, off_t *psize) +{ + if (dbf->file_size == -1) + { + struct stat sb; + if (fstat (dbf->desc, &sb)) + { + GDBM_SET_ERRNO (dbf, GDBM_FILE_STAT_ERROR, FALSE); + return -1; + } + dbf->file_size = sb.st_size; + } + *psize = dbf->file_size; + return 0; +} diff --git a/src/mmap.c b/src/mmap.c index 354fee3..8d5e6c6 100644 --- a/src/mmap.c +++ b/src/mmap.c @@ -61,21 +61,6 @@ SUM_FILE_SIZE (GDBM_FILE dbf, off_t delta) return -1; } -/* Store the size of the GDBM file DBF in *PSIZE. - Return 0 on success and -1 on failure. */ -int -_gdbm_file_size (GDBM_FILE dbf, off_t *psize) -{ - struct stat sb; - if (fstat (dbf->desc, &sb)) - { - GDBM_SET_ERRNO (dbf, GDBM_FILE_STAT_ERROR, FALSE); - return -1; - } - *psize = sb.st_size; - return 0; -} - /* Unmap the region. Reset all mapped fields to initial values. */ void _gdbm_mapped_unmap (GDBM_FILE dbf) diff --git a/src/proto.h b/src/proto.h index c4b1a08..2b34ae3 100644 --- a/src/proto.h +++ b/src/proto.h @@ -54,6 +54,8 @@ int gdbm_bucket_avail_table_validate (GDBM_FILE dbf, hash_bucket *bucket); int _gdbm_validate_header (GDBM_FILE dbf); +int _gdbm_file_size (GDBM_FILE dbf, off_t *psize); + /* From mmap.c */ int _gdbm_mapped_init (GDBM_FILE); void _gdbm_mapped_unmap (GDBM_FILE); -- cgit v1.2.1