diff --git a/ext/dba/libflatfile/flatfile.c b/ext/dba/libflatfile/flatfile.c index 561766777f6f..a5a9bf70c23b 100644 --- a/ext/dba/libflatfile/flatfile.c +++ b/ext/dba/libflatfile/flatfile.c @@ -35,6 +35,18 @@ #define FLATFILE_BLOCK_SIZE 1024 +/* Parse the length prefix in `buf` into `num` and grow `buf` to hold it. + * atoi() narrows a malformed (e.g. negative) length to a huge size_t whose + * `+ FLATFILE_BLOCK_SIZE` would overflow erealloc(); the macro yields true in + * that case so the caller stops reading and the read stays within `buf_size`. */ +#define FLATFILE_GROW_BUF(num, buf, buf_size) ( \ + (num) = atoi(buf), \ + (num) >= (buf_size) && ( \ + (num) > SIZE_MAX - FLATFILE_BLOCK_SIZE \ + || ((buf) = erealloc((buf), (buf_size) = (num) + FLATFILE_BLOCK_SIZE), 0) \ + ) \ +) + /* * ret = -1 means that database was opened for read-only * ret = 0 success @@ -110,10 +122,8 @@ int flatfile_delete(flatfile *dba, datum key_datum) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } pos = php_stream_tell(dba->fp); @@ -133,10 +143,8 @@ int flatfile_delete(flatfile *dba, datum key_datum) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } /* read in the value */ num = php_stream_read(dba->fp, buf, num); @@ -160,10 +168,8 @@ int flatfile_findkey(flatfile *dba, datum key_datum) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); @@ -176,10 +182,8 @@ int flatfile_findkey(flatfile *dba, datum key_datum) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); } @@ -200,10 +204,8 @@ datum flatfile_firstkey(flatfile *dba) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); @@ -216,10 +218,8 @@ datum flatfile_firstkey(flatfile *dba) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); } @@ -242,20 +242,16 @@ datum flatfile_nextkey(flatfile *dba) { if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); if (!php_stream_gets(dba->fp, buf, 15)) { break; } - num = atoi(buf); - if (num >= buf_size) { - buf_size = num + FLATFILE_BLOCK_SIZE; - buf = erealloc(buf, buf_size); + if (FLATFILE_GROW_BUF(num, buf, buf_size)) { + break; } num = php_stream_read(dba->fp, buf, num); diff --git a/ext/dba/tests/dba_flatfile_oob.phpt b/ext/dba/tests/dba_flatfile_oob.phpt new file mode 100644 index 000000000000..3328e1dcba90 --- /dev/null +++ b/ext/dba/tests/dba_flatfile_oob.phpt @@ -0,0 +1,31 @@ +--TEST-- +DBA FlatFile handler bounds with a malformed (negative) length field +--EXTENSIONS-- +dba +--SKIPIF-- + +--FILE-- + +--CLEAN-- + +--EXPECT-- +bool(false) +bool(false) +bool(false) +done