Use std::string for IInput::GetClipboardText

Simplify clipboard handling by returning an `std::string` and freeing the string returned by SDL immediately, so the clipboard data does not stay in memory unnecessarily after the clipboard has been used until the clipboard data is requested again. Fix possible TOCTOU when pasting from the clipboard into a lineinput, due to the clipboard data being requested twice.
Robyt3 · Aug 19, 2024 · 40fbc21 · 40fbc21
1 parent 89cc2d4
commit 40fbc21
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 18 deletions.
diff --git a/src/engine/client/input.cpp b/src/engine/client/input.cpp
@@ -68,8 +68,6 @@ CInput::CInput()
 
 	m_MouseFocus = true;
 
-	m_pClipboardText = nullptr;
-
 	m_CompositionCursor = 0;
 	m_CandidateSelectedIndex = -1;
 
@@ -90,7 +88,6 @@ void CInput::Init()
 
 void CInput::Shutdown()
 {
-	SDL_free(m_pClipboardText);
 	CloseJoysticks();
 }
 
@@ -303,11 +300,12 @@ const std::vector<IInput::CTouchFingerState> &CInput::TouchFingerStates() const
 	return m_vTouchFingerStates;
 }
 
-const char *CInput::GetClipboardText()
+std::string CInput::GetClipboardText()
 {
-	SDL_free(m_pClipboardText);
-	m_pClipboardText = SDL_GetClipboardText();
-	return m_pClipboardText;
+	char *pClipboardText = SDL_GetClipboardText();
+	std::string ClipboardText = pClipboardText;
+	SDL_free(pClipboardText);
+	return ClipboardText;
 }
 
 void CInput::SetClipboardText(const char *pText)

diff --git a/src/engine/client/input.h b/src/engine/client/input.h
@@ -74,7 +74,6 @@ class CInput : public IEngineInput
 	float GetJoystickDeadzone();
 
 	bool m_InputGrabbed;
-	char *m_pClipboardText;
 
 	bool m_MouseFocus;
 #if defined(CONF_PLATFORM_ANDROID)
@@ -151,7 +150,7 @@ class CInput : public IEngineInput
 
 	const std::vector<CTouchFingerState> &TouchFingerStates() const override;
 
-	const char *GetClipboardText() override;
+	std::string GetClipboardText() override;
 	void SetClipboardText(const char *pText) override;
 
 	void StartTextInput() override;

diff --git a/src/engine/input.h b/src/engine/input.h
@@ -10,6 +10,7 @@
 
 #include <cstdint>
 #include <functional>
+#include <string>
 #include <vector>
 
 const int g_MaxKeys = 512;
@@ -138,7 +139,7 @@ class IInput : public IInterface
 	virtual const std::vector<CTouchFingerState> &TouchFingerStates() const = 0;
 
 	// clipboard
-	virtual const char *GetClipboardText() = 0;
+	virtual std::string GetClipboardText() = 0;
 	virtual void SetClipboardText(const char *pText) = 0;
 
 	// text editing

diff --git a/src/game/client/lineinput.cpp b/src/game/client/lineinput.cpp
@@ -331,10 +331,9 @@ bool CLineInput::ProcessInput(const IInput::CEvent &Event)
 		}
 		else if(ModPressed && !AltPressed && Event.m_Key == KEY_V)
 		{
-			const char *pClipboardText = Input()->GetClipboardText();
-			if(pClipboardText)
+			std::string ClipboardText = Input()->GetClipboardText();
+			if(!ClipboardText.empty())
 			{
-				std::string ClipboardText = Input()->GetClipboardText();
 				if(m_pfnClipboardLineCallback)
 				{
 					// Split clipboard text into multiple lines. Send all complete lines to callback.

diff --git a/src/game/editor/editor.cpp b/src/game/editor/editor.cpp
@@ -3665,12 +3665,12 @@ void CEditor::DoColorPickerButton(const void *pId, const CUIRect *pRect, ColorRG
 	{
 		if(ButtonResult == 1)
 		{
-			const char *pClipboard = Input()->GetClipboardText();
-			if(*pClipboard == '#' || *pClipboard == '$') // ignore leading # (web color format) and $ (console color format)
-				++pClipboard;
-			if(str_isallnum_hex(pClipboard))
+			std::string Clipboard = Input()->GetClipboardText();
+			if(Clipboard[0] == '#' || Clipboard[0] == '$') // ignore leading # (web color format) and $ (console color format)
+				Clipboard = Clipboard.substr(1);
+			if(str_isallnum_hex(Clipboard.c_str()))
 			{
-				std::optional<ColorRGBA> ParsedColor = color_parse<ColorRGBA>(pClipboard);
+				std::optional<ColorRGBA> ParsedColor = color_parse<ColorRGBA>(Clipboard.c_str());
 				if(ParsedColor)
 				{
 					m_ColorPickerPopupContext.m_State = EEditState::ONE_GO;