Skip to content

Possible use-after-free of the mouse-callback userdata freed before the window is destroyed in (*Window).Close #1380

Description

@OvOhao

Possible use-after-free of the mouse-callback userdata freed before the window is destroyed in (*Window).Close

I found a possible use-after-free in (*Window).Close. The window name string that
SetMouseHandler registered with OpenCV as the mouse-callback userdata is freed at line 70, but
the call that actually tears down that callback registration — cv::destroyWindow, reached through
C.Window_Close — only runs at line 74. Between those two lines the registration is still live and
points at freed memory, so a mouse event delivered by the HighGUI event thread in that interval
makes go_onmouse_dispatcher run C.GoString on a freed pointer. The ordering defect is
unconditional; observing the dereference requires an event to land inside the window.

File: highgui.go (Go side), highgui.cpp (C++ side)

Function: gocv.io/x/gocv.(*Window).Close (highgui.go:62-79)

func (w *Window) Close() error {
	cName := C.CString(w.name)
	defer C.free(unsafe.Pointer(cName))

	mcbInfo, exists := onMouseHandlers[w.name]
	if exists {
		mcbInfo.fn = nil
		mcbInfo.userdata = nil
		C.free(unsafe.Pointer(mcbInfo.c_name_ptr))
		delete(onMouseHandlers, w.name)
	}

	C.Window_Close(cName)
	w.open = false

	runtime.UnlockOSThread()
	return nil
}

highgui.cpp:5-11 shows that the freed pointer is exactly what OpenCV holds as userdata:

void Window_SetMouseCallback(char* winname, mouse_callback on_mouse) {
    try {
        cv::setMouseCallback(winname, on_mouse, (void*)winname);
    } catch(const cv::Exception& e){
        setExceptionInfo(e.code, e.what());
    }
}

and highgui.go:397-406 is the dispatcher that dereferences it:

//export go_onmouse_dispatcher
func go_onmouse_dispatcher(event C.int, x C.int, y C.int, flags C.int, userdata unsafe.Pointer) {

	c_winname := (*C.char)(unsafe.Pointer(userdata))
	winName := C.GoString(c_winname)
	info, exists := onMouseHandlers[winName]
	if !exists {
		return
	}
	info.fn(int(event), int(x), int(y), int(flags), info.userdata)
}
  1. The application calls win.SetMouseHandler(fn, ud); highgui.go:410 allocates c_winname and
    line 417 hands it to OpenCV.
  2. Window_SetMouseCallback passes that same pointer twice — once as the window name and once as
    the opaque userdata argument to cv::setMouseCallback. OpenCV stores the userdata word
    verbatim in the window's callback record, and that record stays valid until cv::destroyWindow
    runs.
  3. OpenCV delivers mouse events from whichever thread pumps the GUI event loop (the
    cv::waitKey/imshow thread on the HighGUI backend), not necessarily the goroutine calling
    Close.
  4. Close frees the block at line 70 and deletes the Go map entry at line 71, but only calls
    C.Window_Close — i.e. cv::destroyWindow, the operation that removes the callback
    registration — at line 74. Nothing deregisters the callback first; there is no
    cv::setMouseCallback(name, nullptr, nullptr) anywhere in the repository.
  5. A mouse event delivered between line 70 and the completion of line 74 therefore reaches
    go_onmouse_dispatcher with a dangling userdata, and line 400 runs C.GoString on freed
    memory — reading the freed block and scanning it for a NUL byte. Note that the exists check on
    line 401 happens after the freed memory has already been read. If the block has been
    reallocated in the interval, winName may match a different live window's entry and the event
    is dispatched to the wrong handler with the wrong userdata.

Go trigger (if applicable):

win := gocv.NewWindow("demo")
win.SetMouseHandler(func(event, x, y, flags int, ud interface{}) {}, nil)

img := gocv.NewMatWithSize(480, 640, gocv.MatTypeCV8UC3)
defer img.Close()
win.IMShow(img)
win.WaitKey(1)

// Move the mouse continuously over the window while this runs.
win.Close() // highgui.go:70 frees the userdata; highgui.go:74 destroys the window.

This needs concurrency between the GUI event loop and the closing goroutine, which is how
Close/WaitKey are normally used; it does not need GC involvement, since the memory is C-owned.

To be clear about what I have and have not shown. The ordering is unconditional and visible in
the source: Close frees the registered userdata at line 70 and only tears down the registration
at line 74. What I have not done is demonstrate a specific HighGUI backend actually dispatching
a mouse callback inside that window — the interval is short, and whether a given backend (GTK, Qt,
Win32, Cocoa) can deliver an event there depends on its event-pumping model. So this is best read
as a teardown-ordering defect with use-after-free potential rather than a reproduced UAF; a backend
repro, or confirmation from someone familiar with the HighGUI internals, would settle it. The fix
below is worth applying either way, since it costs nothing and removes the window entirely.

Suggested fix: destroy the window (or explicitly deregister the callback) before releasing the user
data — i.e. move the free after C.Window_Close:

func (w *Window) Close() error {
	cName := C.CString(w.name)
	defer C.free(unsafe.Pointer(cName))

	C.Window_Close(cName) // tears down the callback registration first

	if mcbInfo, exists := onMouseHandlers[w.name]; exists {
		C.free(unsafe.Pointer(mcbInfo.c_name_ptr))
		delete(onMouseHandlers, w.name)
	}
	...
}

Using a runtime/cgo.Handle as the callback user data instead of a raw heap pointer would remove
the dangling-pointer window entirely, since a deleted handle fails lookup rather than dereferencing
freed memory.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions