From: Flavien GUEDEZ <flav.pg@oopacity.net>
To: pgsql-bugs@lists.postgresql.org
Subject: Insufficient memory access checks in pglz_decompress
Date: Wed, 18 Oct 2023 16:21:57 +0200
Message-ID: <01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net> (raw)
Hi,
After some investigations about very corrupted toast data in one
postgres instance, I found that the pglz_decompress function (in
common/pg_lzcompress.c) does not check correctly where it copies data
from using memcpy(), which could result in segfault.
In this function, there are other checks to ensure that we do not copy
after the destination end, but not if we copy data from "before the
beginning".
Apologize, I am not a C developer and I am not used to submitting patches.
Though I have tried and attached kind of PoC with a relatively random
corrupted payload (it was beginning with those bytes in my storage for
obscure reasons).
Also attached a simple patch of what could be done just before the
memcpy calls.
Regards,
Flavien
Attachments:
[text/x-patch] check_for_corrupted.patch (822B, ../01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net/2-check_for_corrupted.patch)
download | inline diff:--- a/src/common/pg_lzcompress.c 2023-10-18 12:34:30.616000000 +0000+++ b/src/common/pg_lzcompress.c 2023-10-18 12:48:09.968000000 +0000@@ -761,7 +761,12 @@
/*
* We can safely copy "off" bytes since that clearly
* results in non-overlapping source and destination.
+ * Check for corrupt data: if we fall before the beginning of the+ * destination, we have problems.
*/
+ if (dp - off < dest) {+ return -1;+ }
memcpy(dp, dp - off, off);
len -= off;
dp += off;
@@ -792,6 +797,13 @@
*/
off += off;
}
+ /*+ * Check for corrupt data: if we fall before the beginning of the+ * destination, we have problems.+ */+ if (dp - off < dest) {+ return -1;+ }
memcpy(dp, dp - off, len);
dp += len;
}
[text/x-csrc] poc.c (671B, ../01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net/3-poc.c)
download | inline:
#include <ctype.h>
#include <stdio.h>
#include "postgres.h"
int
main(int argv, char **argc)
{
char *corrupted_compressed_payload = "\x21\x1C\x44\x36\x38\x56\x40\x37\0";
int corrupted_compressed_payload_size = strlen(corrupted_compressed_payload);
int decompress_tmp_buff_size = 1024; // very arbitrary
char *decompress_tmp_buff = malloc(decompress_tmp_buff_size*sizeof(char));
int decompress_ret;
decompress_ret = pglz_decompress(corrupted_compressed_payload, corrupted_compressed_payload_size,
decompress_tmp_buff, decompress_tmp_buff_size, true);
if(decompress_ret == -1) {
printf("data is corrupted\n");
}
free(decompress_tmp_buff);
return 0;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: flav.pg@oopacity.net, pgsql-bugs@lists.postgresql.org
Subject: Re: Insufficient memory access checks in pglz_decompress
In-Reply-To: <01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox