[Spice-commits] 6 commits - common/quic.c common/quic_family_tmpl.c common/quic_tmpl.c tests/fuzzer-quic-testcases tests/test-quic.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Oct 6 12:08:15 UTC 2020
common/quic.c | 15 +++++++++
common/quic_family_tmpl.c | 7 +++-
common/quic_tmpl.c | 6 +++
tests/fuzzer-quic-testcases/test1.quic |binary
tests/fuzzer-quic-testcases/test2.quic |binary
tests/fuzzer-quic-testcases/test3.quic |binary
tests/fuzzer-quic-testcases/test4.quic |binary
tests/test-quic.c | 51 ++++++++++++++++++++++++++++++++-
8 files changed, 75 insertions(+), 4 deletions(-)
New commits:
commit d589542e0492888ac1b300200c7c6cb4eaf88cb0
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Thu Apr 30 07:55:15 2020 +0100
test-quic: Add test cases for quic fuzzer
To use for start for the fuzzer.
Tests have been generated with a patch like:
diff --git a/tests/test-quic.c b/tests/test-quic.c
--- a/tests/test-quic.c
+++ b/tests/test-quic.c
@@ -372,8 +372,8 @@ static void pixbuf_compare(GdkPixbuf *pixbuf_a, GdkPixbuf *pixbuf_b)
static GdkPixbuf *pixbuf_new_random(int alpha)
{
gboolean has_alpha = alpha >= 0 ? alpha : g_random_boolean();
- gint width = g_random_int_range(100, 2000);
- gint height = g_random_int_range(100, 500);
+ gint width = g_random_int_range(10, 100);
+ gint height = g_random_int_range(10, 100);
GdkPixbuf *random_pixbuf;
guint i, size;
guint8 *pixels;
@@ -401,6 +401,12 @@ static void test_pixbuf(GdkPixbuf *pixbuf)
compressed_data = quic_encode_from_pixbuf(pixbuf, imgbuf);
uncompressed_pixbuf = quic_decode_to_pixbuf(compressed_data);
+ {
+ static int num = 0;
+ char fn[256];
+ sprintf(fn, "test%d.quic", ++num);
+ g_assert(g_file_set_contents(fn, (void *) compressed_data->data, compressed_data->len, NULL));
+ }
image_buf_free(imgbuf, uncompressed_pixbuf);
//g_assert(memcmp(gdk_pixbuf_get_pixels(pixbuf), gdk_pixbuf_get_pixels(uncompressed_pixbuf), gdk_pixbuf_get_byte_length(uncompressed_pixbuf)));
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/tests/fuzzer-quic-testcases/test1.quic b/tests/fuzzer-quic-testcases/test1.quic
new file mode 100644
index 0000000..e5490de
Binary files /dev/null and b/tests/fuzzer-quic-testcases/test1.quic differ
diff --git a/tests/fuzzer-quic-testcases/test2.quic b/tests/fuzzer-quic-testcases/test2.quic
new file mode 100644
index 0000000..ed1a7f8
Binary files /dev/null and b/tests/fuzzer-quic-testcases/test2.quic differ
diff --git a/tests/fuzzer-quic-testcases/test3.quic b/tests/fuzzer-quic-testcases/test3.quic
new file mode 100644
index 0000000..5241714
Binary files /dev/null and b/tests/fuzzer-quic-testcases/test3.quic differ
diff --git a/tests/fuzzer-quic-testcases/test4.quic b/tests/fuzzer-quic-testcases/test4.quic
new file mode 100644
index 0000000..fff7251
Binary files /dev/null and b/tests/fuzzer-quic-testcases/test4.quic differ
commit 3b81e67979a35db498c1db8973c503709c7328d7
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Wed Apr 29 15:13:43 2020 +0100
test-quic: Add fuzzer capabilities to the test
Allows it to be used for fuzzying compressed images.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/tests/test-quic.c b/tests/test-quic.c
index 7af6a68..01f159b 100644
--- a/tests/test-quic.c
+++ b/tests/test-quic.c
@@ -14,6 +14,20 @@
You should have received a copy of the GNU Lesser General Public
License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+
+/* Test QUIC encoding and decoding. This test can also be used to fuzz the decoding.
+ *
+ * To use for the fuzzer you should:
+ * 1- build enabling AFL.
+ * $ make clean
+ * $ make CC=afl-gcc CFLAGS='-O2 -fno-omit-frame-pointer'
+ * 2- run AFL, the export is to use ElectricFence to detect some additional
+ * possible buffer overflow, AFL required the program to crash in case of errors
+ * $ cd tests
+ * $ mkdir afl_findings
+ * $ export AFL_PRELOAD=/usr/lib64/libefence.so.0.0
+ * $ afl-fuzz -i fuzzer-quic-testcases -o afl_findings -m 100 -- ./test_quic --fuzzer-decode @@
+ */
#include <config.h>
#include <stdlib.h>
@@ -32,6 +46,7 @@ typedef enum {
} color_mode_t;
static color_mode_t color_mode = COLOR_MODE_RGB;
+static bool fuzzying = false;
typedef struct {
QuicUsrContext usr;
@@ -41,6 +56,10 @@ typedef struct {
static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
{
+ if (fuzzying) {
+ exit(1);
+ }
+
va_list ap;
va_start(ap, fmt);
@@ -300,10 +319,14 @@ static GdkPixbuf *quic_decode_to_pixbuf(GByteArray *compressed_data)
status = quic_decode_begin(quic,
(uint32_t *)compressed_data->data, compressed_data->len/4,
&type, &width, &height);
+ /* limit size for fuzzer, he restrict virtual memory */
+ if (fuzzying && (status != QUIC_OK || (width * height) > 16 * 1024 * 1024 / 4)) {
+ exit(1);
+ }
g_assert(status == QUIC_OK);
pixbuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB,
- (type == QUIC_IMAGE_TYPE_RGBA), 8,
+ (type == QUIC_IMAGE_TYPE_RGBA || type == QUIC_IMAGE_TYPE_RGB32), 8,
width, height);
status = quic_decode(quic, type,
gdk_pixbuf_get_pixels(pixbuf),
@@ -391,8 +414,34 @@ static void test_pixbuf(GdkPixbuf *pixbuf)
}
+static int
+fuzzer_decode(const char *fn)
+{
+ GdkPixbuf *uncompressed_pixbuf;
+ GByteArray compressed_data[1];
+ gchar *contents = NULL;
+ gsize length;
+
+ fuzzying = true;
+ if (!g_file_get_contents(fn, &contents, &length, NULL)) {
+ exit(1);
+ }
+ compressed_data->data = (void*) contents;
+ compressed_data->len = length;
+ uncompressed_pixbuf = quic_decode_to_pixbuf(compressed_data);
+
+ g_object_unref(uncompressed_pixbuf);
+ g_free(contents);
+
+ return 0;
+}
+
int main(int argc, char **argv)
{
+ if (argc >= 3 && strcmp(argv[1], "--fuzzer-decode") == 0) {
+ return fuzzer_decode(argv[2]);
+ }
+
if (argc >= 2) {
for (int i = 1; i < argc; ++i) {
GdkPixbuf *source_pixbuf;
commit b24fe6b66b86e601c725d30f00c37e684b6395b6
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Thu Apr 30 10:19:09 2020 +0100
quic: Avoid possible buffer overflow in find_bucket
Proved by fuzzing the code.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/common/quic_family_tmpl.c b/common/quic_family_tmpl.c
index 8a5f7d2..6cc051b 100644
--- a/common/quic_family_tmpl.c
+++ b/common/quic_family_tmpl.c
@@ -103,7 +103,12 @@ static s_bucket *FNAME(find_bucket)(Channel *channel, const unsigned int val)
{
spice_extra_assert(val < (0x1U << BPC));
- return channel->_buckets_ptrs[val];
+ /* The and (&) here is to avoid buffer overflows in case of garbage or malicious
+ * attempts. Is much faster then using comparisons and save us from such situations.
+ * Note that on normal build the check above won't be compiled as this code path
+ * is pretty hot and would cause speed regressions.
+ */
+ return channel->_buckets_ptrs[val & ((1U << BPC) - 1)];
}
#undef FNAME
commit ef1b6ff7b82e15d759e5415b8e35b92bb1a4c206
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Wed Apr 29 15:11:38 2020 +0100
quic: Check RLE lengths
Avoid buffer overflows decoding images. On compression we compute
lengths till end of line so it won't cause regressions.
Proved by fuzzing the code.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/common/quic_tmpl.c b/common/quic_tmpl.c
index ecd6f3f..ebae992 100644
--- a/common/quic_tmpl.c
+++ b/common/quic_tmpl.c
@@ -563,7 +563,11 @@ static void FNAME_DECL(uncompress_row_seg)(const PIXEL * const prev_row,
do_run:
state->waitcnt = stopidx - i;
run_index = i;
- run_end = i + decode_state_run(encoder, state);
+ run_end = decode_state_run(encoder, state);
+ if (run_end < 0 || run_end > (end - i)) {
+ encoder->usr->error(encoder->usr, "wrong RLE\n");
+ }
+ run_end += i;
for (; i < run_end; i++) {
UNCOMPRESS_PIX_START(&cur_row[i]);
commit 404d74782c8b5e57d146c5bf3118bb41bf3378e4
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Wed Apr 29 15:10:24 2020 +0100
quic: Check image size in quic_decode_begin
Avoid some overflow in code due to images too big or
negative numbers.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/common/quic.c b/common/quic.c
index bc753ca..6815316 100644
--- a/common/quic.c
+++ b/common/quic.c
@@ -56,6 +56,9 @@ typedef uint8_t BYTE;
#define MINwminext 1
#define MAXwminext 100000000
+/* Maximum image size in pixels, mainly to avoid possible integer overflows */
+#define SPICE_MAX_IMAGE_SIZE (512 * 1024 * 1024 - 1)
+
typedef struct QuicFamily {
unsigned int nGRcodewords[MAXNUMCODES]; /* indexed by code number, contains number of
unmodified GR codewords in the code */
@@ -1165,6 +1168,16 @@ int quic_decode_begin(QuicContext *quic, uint32_t *io_ptr, unsigned int num_io_w
height = encoder->io_word;
decode_eat32bits(encoder);
+ if (width <= 0 || height <= 0) {
+ encoder->usr->warn(encoder->usr, "invalid size\n");
+ return QUIC_ERROR;
+ }
+
+ /* avoid too big images */
+ if ((uint64_t) width * height > SPICE_MAX_IMAGE_SIZE) {
+ encoder->usr->error(encoder->usr, "image too large\n");
+ }
+
quic_image_params(encoder, type, &channels, &bpc);
if (!encoder_reset_channels(encoder, channels, width, bpc)) {
commit 762e0abae36033ccde658fd52d3235887b60862d
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Wed Apr 29 15:09:13 2020 +0100
quic: Check we have some data to start decoding quic image
All paths already pass some data to quic_decode_begin but for the
test check it, it's not that expensive test.
Checking for not 0 is enough, all other words will potentially be
read calling more_io_words but we need one to avoid a potential
initial buffer overflow or deferencing an invalid pointer.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Uri Lublin <uril at redhat.com>
diff --git a/common/quic.c b/common/quic.c
index e2dee0f..bc753ca 100644
--- a/common/quic.c
+++ b/common/quic.c
@@ -1136,7 +1136,7 @@ int quic_decode_begin(QuicContext *quic, uint32_t *io_ptr, unsigned int num_io_w
int channels;
int bpc;
- if (!encoder_reset(encoder, io_ptr, io_ptr_end)) {
+ if (!num_io_words || !encoder_reset(encoder, io_ptr, io_ptr_end)) {
return QUIC_ERROR;
}
More information about the Spice-commits
mailing list