From 5e46e3380887dcd30dc7ea582496bbd9a6a4d70c Mon Sep 17 00:00:00 2001 From: Albrecht Schlosser Date: Fri, 1 Jul 2022 18:49:57 +0200 Subject: [PATCH] Fix "Segfault if using very large selections" (issue 451) Backported from 'master' (fltk 1.4.0). For more information see commit c5556291624eec58ed9de186474dfcc858dca691 and issue 451 https://github.com/fltk/fltk/issues/451 --- src/Fl_x.cxx | 66 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/src/Fl_x.cxx b/src/Fl_x.cxx index 7c03d33..c8b9c06 100644 --- a/src/Fl_x.cxx +++ b/src/Fl_x.cxx @@ -1273,19 +1273,24 @@ static bool getNextEvent(XEvent *event_return) return true; } -static long getIncrData(uchar* &data, const XSelectionEvent& selevent, long lower_bound) -{ -//fprintf(stderr,"Incremental transfer starting due to INCR property\n"); +static long getIncrData(uchar* &data, const XSelectionEvent& selevent, size_t lower_bound) { + // fprintf(stderr,"Incremental transfer starting due to INCR property\n"); size_t total = 0; XEvent event; - XDeleteProperty(fl_display, selevent.requestor, selevent.property); + XDeleteProperty(fl_display, selevent.requestor, selevent.property); data = (uchar*)realloc(data, lower_bound); - for (;;) - { - if (!getNextEvent(&event)) break; - if (event.type == PropertyNotify) - { - if (event.xproperty.state != PropertyNewValue) continue; + if (!data) { + // fprintf(stderr, "[getIncrData:%d] realloc() FAILED, size = %ld\n", __LINE__, lower_bound); + Fl::fatal("Clipboard data transfer failed, size %ld too large.", lower_bound); + } + for (;;) { + if (!getNextEvent(&event)) { + // This is unexpected but may happen if the sender (clipboard owner) no longer sends data + // fprintf(stderr, "[getIncrData:%d] Failed to get next event (timeout) *** break! ***\n", __LINE__); + break; + } + if (event.type == PropertyNotify) { + if (event.xproperty.state != PropertyNewValue) continue; // ignore PropertyDelete Atom actual_type; int actual_format; unsigned long nitems; @@ -1300,15 +1305,39 @@ static long getIncrData(uchar* &data, const XSelectionEvent& selevent, long lowe AnyPropertyType, &actual_type, &actual_format, &nitems, &bytes_after, &prop); num_bytes = nitems * (actual_format / 8); offset += num_bytes/4; - //slice_size += num_bytes; - if (total + num_bytes > (size_t)lower_bound) data = (uchar*)realloc(data, total + num_bytes); - memcpy(data + total, prop, num_bytes); total += num_bytes; + // slice_size += num_bytes; + if (total + num_bytes > lower_bound) { + data = (uchar*)realloc(data, total + num_bytes); + if (!data) { + // fprintf(stderr, "[getIncrData():%d] realloc() FAILED, size = %ld\n", __LINE__, total + num_bytes); + Fl::fatal("Clipboard data transfer failed, size %ld too large.", total + num_bytes); + } + } + memcpy(data + total, prop, num_bytes); + total += num_bytes; if (prop) XFree(prop); } while (bytes_after != 0); //fprintf(stderr,"INCR data size:%ld\n", slice_size); if (num_bytes == 0) break; } - else break; + else { + // Unexpected next event. At this point we're handling the INCR protocol and can't deal with + // *some* other events due to potential recursions. We *could* call fl_handle(event) to handle + // *selected* other events but for the time being we ignore all other events! + // Handling the INCR protocol for very large data may take some time and multiple events. + // Interleaving "other" events are possible, for instance the KeyRelease event of the + // ctrl/v key pressed to insert the clipboard. This solution is not perfect but it can + // handle the INCR protocol with very large selections in most cases, although with potential + // side effects because other events may be ignored. + // See GitHub Issue #451: "Segfault if using very large selections". + // Note: the "fix" for Issue #451 is basically to use 'continue' rather than 'break' + // Debug: + // fprintf(stderr, + // "[getIncrData:%d] getNextEvent() returned %d, not PropertyNotify (%d). Event ignored.\n", + // __LINE__, event.type, PropertyNotify); + + continue; + } } XDeleteProperty(fl_display, selevent.requestor, selevent.property); return (long)total; @@ -1376,7 +1405,9 @@ int fl_handle(const XEvent& thisevent) case SelectionNotify: { static unsigned char* sn_buffer = 0; //static const char *buffer_format = 0; - if (sn_buffer) {XFree(sn_buffer); sn_buffer = 0;} + if (sn_buffer) { + free(sn_buffer); sn_buffer = 0; + } long bytesread = 0; if (fl_xevent->xselection.property) for (;;) { // The Xdnd code pastes 64K chunks together, possibly to avoid @@ -1457,7 +1488,10 @@ fprintf(stderr,"\n");*/ return true; } if (actual == fl_INCR) { - bytesread = getIncrData(sn_buffer, xevent.xselection, *(long*)portion); + // an X11 "integer" (32 bit), the "lower bound" of the clipboard size (see ICCCM) + size_t lower_bound = (*(unsigned long *)portion) & 0xFFFFFFFF; + // fprintf(stderr, "[fl_handle:%d] INCR: lower_bound = %ld\n", __LINE__, lower_bound); + bytesread = getIncrData(sn_buffer, xevent.xselection, lower_bound); XFree(portion); break; }