Changeset - r28650:8f5801871555
[Not reviewed]
master
0 1 0
Patric Stout - 3 months ago 2024-02-02 22:01:54
truebrain@openttd.org
Fix: don't use non-owning string pointer in StringParameter (#11952)

The string pointer can become invalid before the reference is
dropped, causing out-of-bound access in windows like ErrorWindow,
or News that copy 10 or 20 parameters for their internals.

Co-authored-by: Jonathan G Rennison <j.g.rennison@gmail.com>
1 file changed with 4 insertions and 8 deletions:
0 comments (0 inline, 0 general)
src/strings_internal.h
Show inline comments
 
@@ -16,7 +16,6 @@
 
/** The data required to format and validate a single parameter of a string. */
 
struct StringParameter {
 
	uint64_t data; ///< The data of the parameter.
 
	const char *string_view; ///< The string value, if it has any.
 
	std::unique_ptr<std::string> string; ///< Copied string value, if it has any.
 
	char32_t type; ///< The #StringControlCode to interpret this data with when it's the first parameter, otherwise '\0'.
 
};
 
@@ -106,7 +105,7 @@ public:
 
	const char *GetNextParameterString()
 
	{
 
		auto ptr = GetNextParameterPointer();
 
		return ptr->string != nullptr ? ptr->string->c_str() : ptr->string_view;
 
		return ptr->string != nullptr ? ptr->string->c_str() : nullptr;
 
	}
 

	
 
	/**
 
@@ -152,7 +151,6 @@ public:
 
		assert(n < this->parameters.size());
 
		this->parameters[n].data = v;
 
		this->parameters[n].string.reset();
 
		this->parameters[n].string_view = nullptr;
 
	}
 

	
 
	template <typename T, std::enable_if_t<std::is_base_of<StrongTypedefBase, T>::value, int> = 0>
 
@@ -165,8 +163,7 @@ public:
 
	{
 
		assert(n < this->parameters.size());
 
		this->parameters[n].data = 0;
 
		this->parameters[n].string.reset();
 
		this->parameters[n].string_view = str;
 
		this->parameters[n].string = std::make_unique<std::string>(str);
 
	}
 

	
 
	void SetParam(size_t n, const std::string &str) { this->SetParam(n, str.c_str()); }
 
@@ -176,13 +173,12 @@ public:
 
		assert(n < this->parameters.size());
 
		this->parameters[n].data = 0;
 
		this->parameters[n].string = std::make_unique<std::string>(std::move(str));
 
		this->parameters[n].string_view = nullptr;
 
	}
 

	
 
	uint64_t GetParam(size_t n) const
 
	{
 
		assert(n < this->parameters.size());
 
		assert(this->parameters[n].string_view == nullptr && this->parameters[n].string == nullptr);
 
		assert(this->parameters[n].string == nullptr);
 
		return this->parameters[n].data;
 
	}
 

	
 
@@ -195,7 +191,7 @@ public:
 
	{
 
		assert(n < this->parameters.size());
 
		auto &param = this->parameters[n];
 
		return param.string != nullptr ? param.string->c_str() : param.string_view;
 
		return param.string != nullptr ? param.string->c_str() : nullptr;
 
	}
 
};
 

	
0 comments (0 inline, 0 general)