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

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.