clipboard_monitor_start triggers an Operation Error 1247 if accidentally started the second time
wchen-r7 opened this issue · 2 comments
So I am playing w/ the clipboard_monitor commands again. Honestly it works really well, so this one is kind of a nitpick. I would consider this issue as low priority.
If you do this:
meterpreter > clipboard_monitor_start
[+] Clipboard monitor started
meterpreter > clipboard_monitor_start
[-] extapi_clipboard_monitor_start: Operation failed: 1247
meterpreter >
As you can see you get that operation error. At that point if you do clipboard_monitor_stop, you will see a different op error: 758.
Maybe you're wondering "sinn3r why would you do that?" I dunno, cuz I am an asshole?
Even though you see the error, it's still not that bad. You can simply start again and the monitor is back in business.
So I guess it would be nicer to have something that checks if there is already a monitor or not. Or maybe it does already? I haven't tried to look at the code to find out. A print_warning that says "Go Home, You Are Drunk!" or something like that would be great.
Interesting. If you run clipboard_monitor_start a second time, it does actually check to see if it's already running. You can see that check here: https://github.com/rapid7/meterpreter/blob/master/source/extensions/extapi/clipboard.c#L1213
The 1247 represents the ERROR_ALREADY_INITIALIZED error code that you can see returned as part of the condition.
The subsequent call to clipboard_monitor_stop results in 758. This means that it's running into this code path here: https://github.com/rapid7/meterpreter/blob/master/source/extensions/extapi/clipboard.c#L1397 as 758 maps to ERROR_NOTHING_TO_TERMINATE. This implies that the second call to clipboard_monitor_start does something dodgy....
... and it does! Observe: https://github.com/rapid7/meterpreter/blob/master/source/extensions/extapi/clipboard.c#L1271
We can see that gClipboardState is set to NULL this point, which is what causes things to misbehave.
Very nice catch! This is actually not a low priority issue as you have pointed out @wchen-r7, I think it's a higher priority. The reason is because this results in the clipboard monitor running infinitely, which will ultimately blow enough memory to crash (or at least be more obvious forensically). It could also prevent clean shut downs.
I will get on this pronto. Well played, nice find!
Ah I see. Nice analysis.