Codechange: Make sort list function lists safer. (#12574)

GUIList has a pointer only to the start of each sort/filter func list, which has the potential for UB as it is unable to validate that the selected sort or filter type is in range.

Use a std::span instead and check if the selected type is in range before using it.

src/bridge_gui.cpp

@@ -76,7 +76,7 @@ private:
 
 	/* Constants for sorting the bridges */
 	static const StringID sorter_names[];
-	static GUIBridgeList::SortFunction * const sorter_funcs[];
+	static const std::initializer_list<GUIBridgeList::SortFunction * const> sorter_funcs;
 
 	/* Internal variables */
 	TileIndex start_tile;
@@ -298,7 +298,7 @@ public:
 Listing BuildBridgeWindow::last_sorting = {true, 2};
 
 /** Available bridge sorting functions. */
-GUIBridgeList::SortFunction * const BuildBridgeWindow::sorter_funcs[] = {
+const std::initializer_list<GUIBridgeList::SortFunction * const> BuildBridgeWindow::sorter_funcs = {
 	&BridgeIndexSorter,
 	&BridgePriceSorter,
 	&BridgeSpeedSorter

src/industry_gui.cpp

@@ -1326,7 +1326,7 @@ protected:
 
 	/* Constants for sorting industries */
 	static const StringID sorter_names[];
-	static GUIIndustryList::SortFunction * const sorter_funcs[];
+	static const std::initializer_list<GUIIndustryList::SortFunction * const> sorter_funcs;
 
 	GUIIndustryList industries{IndustryDirectoryWindow::produced_cargo_filter};
 	Scrollbar *vscroll;
@@ -1902,7 +1902,7 @@ public:
 Listing IndustryDirectoryWindow::last_sorting = {false, 0};
 
 /* Available station sorting functions. */
-GUIIndustryList::SortFunction * const IndustryDirectoryWindow::sorter_funcs[] = {
+const std::initializer_list<GUIIndustryList::SortFunction * const> IndustryDirectoryWindow::sorter_funcs = {
 	&IndustryNameSorter,
 	&IndustryTypeSorter,
 	&IndustryProductionSorter,

src/network/network_content_gui.cpp

@@ -334,8 +334,8 @@ class NetworkContentListWindow : public Window, ContentCallback {
 
 	static Listing last_sorting;     ///< The last sorting setting.
 	static Filtering last_filtering; ///< The last filtering setting.
-	static GUIContentList::SortFunction * const sorter_funcs[];   ///< Sorter functions
+	static const std::initializer_list<GUIContentList::SortFunction * const> sorter_funcs;   ///< Sorter functions
-	static GUIContentList::FilterFunction * const filter_funcs[]; ///< Filter functions.
+	static const std::initializer_list<GUIContentList::FilterFunction * const> filter_funcs; ///< Filter functions.
 	GUIContentList content;      ///< List with content
 	bool auto_select;            ///< Automatically select all content when the meta-data becomes available
 	ContentListFilterData filter_data; ///< Filter for content list
@@ -1010,13 +1010,13 @@ public:
 Listing NetworkContentListWindow::last_sorting = {false, 1};
 Filtering NetworkContentListWindow::last_filtering = {false, 0};
 
-NetworkContentListWindow::GUIContentList::SortFunction * const NetworkContentListWindow::sorter_funcs[] = {
+const std::initializer_list<NetworkContentListWindow::GUIContentList::SortFunction * const> NetworkContentListWindow::sorter_funcs = {
 	&StateSorter,
 	&TypeSorter,
 	&NameSorter,
 };
 
-NetworkContentListWindow::GUIContentList::FilterFunction * const NetworkContentListWindow::filter_funcs[] = {
+const std::initializer_list<NetworkContentListWindow::GUIContentList::FilterFunction * const> NetworkContentListWindow::filter_funcs = {
 	&TagNameFilter,
 	&TypeOrSelectedFilter,
 };

src/network/network_gui.cpp

@@ -181,8 +181,8 @@ protected:
 	static Listing last_sorting;
 
 	/* Constants for sorting servers */
-	static GUIGameServerList::SortFunction * const sorter_funcs[];
+	static const std::initializer_list<GUIGameServerList::SortFunction * const> sorter_funcs;
-	static GUIGameServerList::FilterFunction * const filter_funcs[];
+	static const std::initializer_list<GUIGameServerList::FilterFunction * const> filter_funcs;
 
 	NetworkGameList *server;        ///< Selected server.
 	NetworkGameList *last_joined;   ///< The last joined server.
@@ -459,8 +459,8 @@ public:
 		this->server = this->last_joined;
 
 		this->servers.SetListing(this->last_sorting);
-		this->servers.SetSortFuncs(this->sorter_funcs);
+		this->servers.SetSortFuncs(NetworkGameWindow::sorter_funcs);
-		this->servers.SetFilterFuncs(this->filter_funcs);
+		this->servers.SetFilterFuncs(NetworkGameWindow::filter_funcs);
 		this->servers.ForceRebuild();
 	}
 
@@ -860,7 +860,7 @@ public:
 };
 
 Listing NetworkGameWindow::last_sorting = {false, 5};
-GUIGameServerList::SortFunction * const NetworkGameWindow::sorter_funcs[] = {
+const std::initializer_list<GUIGameServerList::SortFunction * const> NetworkGameWindow::sorter_funcs = {
 	&NGameNameSorter,
 	&NGameClientSorter,
 	&NGameMapSizeSorter,
@@ -869,7 +869,7 @@ GUIGameServerList::SortFunction * const NetworkGameWindow::sorter_funcs[] = {
 	&NGameAllowedSorter
 };
 
-GUIGameServerList::FilterFunction * const NetworkGameWindow::filter_funcs[] = {
+const std::initializer_list<GUIGameServerList::FilterFunction * const> NetworkGameWindow::filter_funcs = {
 	&NGameSearchFilter
 };
 

src/newgrf_gui.cpp

@@ -609,8 +609,8 @@ struct NewGRFWindow : public Window, NewGRFScanCallback {
 
 	static Listing   last_sorting;   ///< Default sorting of #GUIGRFConfigList.
 	static Filtering last_filtering; ///< Default filtering of #GUIGRFConfigList.
-	static GUIGRFConfigList::SortFunction   * const sorter_funcs[]; ///< Sort functions of the #GUIGRFConfigList.
+	static const std::initializer_list<GUIGRFConfigList::SortFunction   * const> sorter_funcs; ///< Sort functions of the #GUIGRFConfigList.
-	static GUIGRFConfigList::FilterFunction * const filter_funcs[]; ///< Filter functions of the #GUIGRFConfigList.
+	static const std::initializer_list<GUIGRFConfigList::FilterFunction * const> filter_funcs; ///< Filter functions of the #GUIGRFConfigList.
 
 	GUIGRFConfigList avails;    ///< Available (non-active) grfs.
 	const GRFConfig *avail_sel; ///< Currently selected available grf. \c nullptr is none is selected.
@@ -1586,11 +1586,11 @@ void ShowMissingContentWindow(const GRFConfig *list)
 Listing NewGRFWindow::last_sorting     = {false, 0};
 Filtering NewGRFWindow::last_filtering = {false, 0};
 
-NewGRFWindow::GUIGRFConfigList::SortFunction * const NewGRFWindow::sorter_funcs[] = {
+const std::initializer_list<NewGRFWindow::GUIGRFConfigList::SortFunction * const> NewGRFWindow::sorter_funcs = {
 	&NameSorter,
 };
 
-NewGRFWindow::GUIGRFConfigList::FilterFunction * const NewGRFWindow::filter_funcs[] = {
+const std::initializer_list<NewGRFWindow::GUIGRFConfigList::FilterFunction * const> NewGRFWindow::filter_funcs = {
 	&TagNameFilter,
 };
 

src/object_gui.cpp

@@ -56,8 +56,8 @@ class BuildObjectWindow : public Window {
 
 	static Listing   last_sorting;         ///< Default sorting of #GUIObjectClassList.
 	static Filtering last_filtering;       ///< Default filtering of #GUIObjectClassList.
-	static GUIObjectClassList::SortFunction   * const sorter_funcs[]; ///< Sort functions of the #GUIObjectClassList.
+	static const std::initializer_list<GUIObjectClassList::SortFunction   * const> sorter_funcs; ///< Sort functions of the #GUIObjectClassList.
-	static GUIObjectClassList::FilterFunction * const filter_funcs[]; ///< Filter functions of the #GUIObjectClassList.
+	static const std::initializer_list<GUIObjectClassList::FilterFunction * const> filter_funcs; ///< Filter functions of the #GUIObjectClassList.
 	GUIObjectClassList object_classes;     ///< Available object classes.
 	StringFilter string_filter;            ///< Filter for available objects.
 	QueryString filter_editbox;            ///< Filter editbox.
@@ -655,11 +655,11 @@ public:
 Listing BuildObjectWindow::last_sorting = { false, 0 };
 Filtering BuildObjectWindow::last_filtering = { false, 0 };
 
-BuildObjectWindow::GUIObjectClassList::SortFunction * const BuildObjectWindow::sorter_funcs[] = {
+const std::initializer_list<BuildObjectWindow::GUIObjectClassList::SortFunction * const> BuildObjectWindow::sorter_funcs = {
 	&ObjectClassIDSorter,
 };
 
-BuildObjectWindow::GUIObjectClassList::FilterFunction * const BuildObjectWindow::filter_funcs[] = {
+const std::initializer_list<BuildObjectWindow::GUIObjectClassList::FilterFunction * const> BuildObjectWindow::filter_funcs = {
 	&TagNameFilter,
 };
 

src/rail_gui.cpp

@@ -964,8 +964,8 @@ private:
 
 	static Listing   last_sorting;           ///< Default sorting of #GUIStationClassList.
 	static Filtering last_filtering;         ///< Default filtering of #GUIStationClassList.
-	static GUIStationClassList::SortFunction * const sorter_funcs[];   ///< Sort functions of the #GUIStationClassList.
+	static const std::initializer_list<GUIStationClassList::SortFunction * const> sorter_funcs;   ///< Sort functions of the #GUIStationClassList.
-	static GUIStationClassList::FilterFunction * const filter_funcs[]; ///< Filter functions of the #GUIStationClassList.
+	static const std::initializer_list<GUIStationClassList::FilterFunction * const> filter_funcs; ///< Filter functions of the #GUIStationClassList.
 	GUIStationClassList station_classes;     ///< Available station classes.
 	StringFilter string_filter;              ///< Filter for available station classes.
 	QueryString filter_editbox;              ///< Filter editbox.
@@ -1571,11 +1571,11 @@ public:
 Listing BuildRailStationWindow::last_sorting = { false, 0 };
 Filtering BuildRailStationWindow::last_filtering = { false, 0 };
 
-BuildRailStationWindow::GUIStationClassList::SortFunction * const BuildRailStationWindow::sorter_funcs[] = {
+const std::initializer_list<BuildRailStationWindow::GUIStationClassList::SortFunction * const> BuildRailStationWindow::sorter_funcs = {
 	&StationClassIDSorter,
 };
 
-BuildRailStationWindow::GUIStationClassList::FilterFunction * const BuildRailStationWindow::filter_funcs[] = {
+const std::initializer_list<BuildRailStationWindow::GUIStationClassList::FilterFunction * const> BuildRailStationWindow::filter_funcs = {
 	&TagNameFilter,
 };
 

src/road_gui.cpp

@@ -1112,8 +1112,8 @@ private:
 
 	static Listing   last_sorting;           ///< Default sorting of #GUIRoadStopClassList.
 	static Filtering last_filtering;         ///< Default filtering of #GUIRoadStopClassList.
-	static GUIRoadStopClassList::SortFunction * const sorter_funcs[];   ///< Sort functions of the #GUIRoadStopClassList.
+	static const std::initializer_list<GUIRoadStopClassList::SortFunction * const> sorter_funcs;   ///< Sort functions of the #GUIRoadStopClassList.
-	static GUIRoadStopClassList::FilterFunction * const filter_funcs[]; ///< Filter functions of the #GUIRoadStopClassList.
+	static const std::initializer_list<GUIRoadStopClassList::FilterFunction * const> filter_funcs; ///< Filter functions of the #GUIRoadStopClassList.
 	GUIRoadStopClassList roadstop_classes;   ///< Available road stop classes.
 	StringFilter string_filter;              ///< Filter for available road stop classes.
 	QueryString filter_editbox;              ///< Filter editbox.
@@ -1599,11 +1599,11 @@ public:
 Listing BuildRoadStationWindow::last_sorting = { false, 0 };
 Filtering BuildRoadStationWindow::last_filtering = { false, 0 };
 
-BuildRoadStationWindow::GUIRoadStopClassList::SortFunction * const BuildRoadStationWindow::sorter_funcs[] = {
+const std::initializer_list<BuildRoadStationWindow::GUIRoadStopClassList::SortFunction * const> BuildRoadStationWindow::sorter_funcs = {
 	&RoadStopClassIDSorter,
 };
 
-BuildRoadStationWindow::GUIRoadStopClassList::FilterFunction * const BuildRoadStationWindow::filter_funcs[] = {
+const std::initializer_list<BuildRoadStationWindow::GUIRoadStopClassList::FilterFunction * const> BuildRoadStationWindow::filter_funcs = {
 	&TagNameFilter,
 };
 

src/sortlist_type.h

@@ -50,8 +50,8 @@ public:
 	typedef bool CDECL FilterFunction(const T*, F); ///< Signature of filter function.
 
 protected:
-	SortFunction * const *sort_func_list;     ///< the sort criteria functions
+	std::span<SortFunction * const> sort_func_list;     ///< the sort criteria functions
-	FilterFunction * const *filter_func_list; ///< the filter criteria functions
+	std::span<FilterFunction * const> filter_func_list; ///< the filter criteria functions
 	SortListFlags flags;                      ///< used to control sorting/resorting/etc.
 	uint8_t sort_type;                          ///< what criteria to sort on
 	uint8_t filter_type;                        ///< what criteria to filter on
@@ -85,8 +85,8 @@ public:
 	/* If sort parameters are not used then we don't require a reference to the params. */
 	template <typename T_ = T, typename P_ = P, typename _F = F, std::enable_if_t<std::is_same_v<P_, std::nullptr_t>>* = nullptr>
 	GUIList() :
-		sort_func_list(nullptr),
+		sort_func_list({}),
-		filter_func_list(nullptr),
+		filter_func_list({}),
 		flags(VL_NONE),
 		sort_type(0),
 		filter_type(0),
@@ -97,8 +97,8 @@ public:
 	/* If sort parameters are used then we require a reference to the params. */
 	template <typename T_ = T, typename P_ = P, typename _F = F, std::enable_if_t<!std::is_same_v<P_, std::nullptr_t>>* = nullptr>
 	GUIList(const P &params) :
-		sort_func_list(nullptr),
+		sort_func_list({}),
-		filter_func_list(nullptr),
+		filter_func_list({}),
 		flags(VL_NONE),
 		sort_type(0),
 		filter_type(0),
@@ -123,6 +123,7 @@ public:
 	 */
 	void SetSortType(uint8_t n_type)
 	{
+		assert(n_type < std::size(this->sort_func_list));
 		if (this->sort_type != n_type) {
 			SETBITS(this->flags, VL_RESORT);
 			this->sort_type = n_type;
@@ -175,6 +176,7 @@ public:
 	 */
 	void SetFilterType(uint8_t n_type)
 	{
+		assert(n_type < std::size(this->filter_func_list));
 		if (this->filter_type != n_type) {
 			this->filter_type = n_type;
 		}
@@ -288,11 +290,11 @@ public:
 	}
 
 	/**
-	 * Hand the array of sort function pointers to the sort list
+	 * Hand the sort function pointers to the GUIList.
 	 *
-	 * @param n_funcs The pointer to the first sort func
+	 * @param n_funcs Span covering the sort function pointers.
 	 */
-	void SetSortFuncs(SortFunction * const *n_funcs)
+	void SetSortFuncs(std::span<SortFunction * const> n_funcs)
 	{
 		this->sort_func_list = n_funcs;
 	}
@@ -305,7 +307,8 @@ public:
 	 */
 	bool Sort()
 	{
-		assert(this->sort_func_list != nullptr);
+		if (this->sort_func_list.empty()) return false;
+		assert(this->sort_type < this->sort_func_list.size());
 		return this->Sort(this->sort_func_list[this->sort_type]);
 	}
 
@@ -359,11 +362,11 @@ public:
 	}
 
 	/**
-	 * Hand the array of filter function pointers to the sort list
+	 * Hand the filter function pointers to the GUIList.
 	 *
-	 * @param n_funcs The pointer to the first filter func
+	 * @param n_funcs Span covering the filter function pointers.
 	 */
-	void SetFilterFuncs(FilterFunction * const *n_funcs)
+	void SetFilterFuncs(std::span<FilterFunction * const> n_funcs)
 	{
 		this->filter_func_list = n_funcs;
 	}
@@ -376,7 +379,8 @@ public:
 	 */
 	bool Filter(F filter_data)
 	{
-		if (this->filter_func_list == nullptr) return false;
+		if (this->filter_func_list.empty()) return false;
+		assert(this->filter_type < this->filter_func_list.size());
 		return this->Filter(this->filter_func_list[this->filter_type], filter_data);
 	}
 

src/station_gui.cpp

@@ -245,7 +245,7 @@ protected:
 
 	/* Constants for sorting stations */
 	static const StringID sorter_names[];
-	static GUIStationList::SortFunction * const sorter_funcs[];
+	static const std::initializer_list<GUIStationList::SortFunction * const> sorter_funcs;
 
 	FilterState filter;
 	GUIStationList stations{filter.cargoes};
@@ -720,7 +720,7 @@ public:
 };
 
 /* Available station sorting functions */
-GUIStationList::SortFunction * const CompanyStationsWindow::sorter_funcs[] = {
+const std::initializer_list<GUIStationList::SortFunction * const> CompanyStationsWindow::sorter_funcs = {
 	&StationNameSorter,
 	&StationTypeSorter,
 	&StationWaitingTotalSorter,

src/story_gui.cpp

@@ -62,8 +62,8 @@ protected:
 
 	StoryPageElementID active_button_id; ///< Which button element the player is currently using
 
-	static GUIStoryPageList::SortFunction * const page_sorter_funcs[];
+	static const std::initializer_list<GUIStoryPageList::SortFunction * const> page_sorter_funcs;
-	static GUIStoryPageElementList::SortFunction * const page_element_sorter_funcs[];
+	static const std::initializer_list<GUIStoryPageElementList::SortFunction * const> page_element_sorter_funcs;
 
 	/** (Re)Build story page list. */
 	void BuildStoryPageList()
@@ -939,11 +939,11 @@ public:
 	}
 };
 
-GUIStoryPageList::SortFunction * const StoryBookWindow::page_sorter_funcs[] = {
+const std::initializer_list<GUIStoryPageList::SortFunction * const> StoryBookWindow::page_sorter_funcs = {
 	&PageOrderSorter,
 };
 
-GUIStoryPageElementList::SortFunction * const StoryBookWindow::page_element_sorter_funcs[] = {
+const std::initializer_list<GUIStoryPageElementList::SortFunction * const> StoryBookWindow::page_element_sorter_funcs = {
 	&PageElementOrderSorter,
 };
 

src/town_gui.cpp

@@ -721,7 +721,7 @@ private:
 
 	/* Constants for sorting towns */
 	static const StringID sorter_names[];
-	static GUITownList::SortFunction * const sorter_funcs[];
+	static const std::initializer_list<GUITownList::SortFunction * const> sorter_funcs;
 
 	StringFilter string_filter;             ///< Filter for towns
 	QueryString townname_editbox;           ///< Filter editbox
@@ -1054,7 +1054,7 @@ const StringID TownDirectoryWindow::sorter_names[] = {
 };
 
 /** Available town directory sorting functions. */
-GUITownList::SortFunction * const TownDirectoryWindow::sorter_funcs[] = {
+const std::initializer_list<GUITownList::SortFunction * const> TownDirectoryWindow::sorter_funcs = {
 	&TownNameSorter,
 	&TownPopulationSorter,
 	&TownRatingSorter,

src/vehicle_gui.cpp

@@ -78,7 +78,7 @@ static bool VehicleIndividualToGroupSorterWrapper(GUIVehicleGroup const &a, GUIV
 	return func(*(a.vehicles_begin), *(b.vehicles_begin));
 }
 
-BaseVehicleListWindow::VehicleGroupSortFunction * const BaseVehicleListWindow::vehicle_group_none_sorter_funcs[] = {
+const std::initializer_list<BaseVehicleListWindow::VehicleGroupSortFunction * const> BaseVehicleListWindow::vehicle_group_none_sorter_funcs = {
 	&VehicleIndividualToGroupSorterWrapper<VehicleNumberSorter>,
 	&VehicleIndividualToGroupSorterWrapper<VehicleNameSorter>,
 	&VehicleIndividualToGroupSorterWrapper<VehicleAgeSorter>,
@@ -128,7 +128,7 @@ const StringID BaseVehicleListWindow::vehicle_group_none_sorter_names_wallclock[
 	INVALID_STRING_ID
 };
 
-BaseVehicleListWindow::VehicleGroupSortFunction * const BaseVehicleListWindow::vehicle_group_shared_orders_sorter_funcs[] = {
+const std::initializer_list<BaseVehicleListWindow::VehicleGroupSortFunction * const> BaseVehicleListWindow::vehicle_group_shared_orders_sorter_funcs = {
 	&VehicleGroupLengthSorter,
 	&VehicleGroupTotalProfitThisYearSorter,
 	&VehicleGroupTotalProfitLastYearSorter,

src/vehicle_gui_base.h

@@ -103,8 +103,8 @@ struct BaseVehicleListWindow : public Window {
 	static const StringID vehicle_group_none_sorter_names_wallclock[];
 	static const StringID vehicle_group_shared_orders_sorter_names_calendar[];
 	static const StringID vehicle_group_shared_orders_sorter_names_wallclock[];
-	static VehicleGroupSortFunction * const vehicle_group_none_sorter_funcs[];
+	static const std::initializer_list<VehicleGroupSortFunction * const> vehicle_group_none_sorter_funcs;
-	static VehicleGroupSortFunction * const vehicle_group_shared_orders_sorter_funcs[];
+	static const std::initializer_list<VehicleGroupSortFunction * const> vehicle_group_shared_orders_sorter_funcs;
 
 	BaseVehicleListWindow(WindowDesc *desc, WindowNumber wno);
 
@@ -126,7 +126,7 @@ struct BaseVehicleListWindow : public Window {
 
 	const StringID *GetVehicleSorterNames();
 
-	VehicleGroupSortFunction * const *GetVehicleSorterFuncs()
+	std::span<VehicleGroupSortFunction * const> GetVehicleSorterFuncs()
 	{
 		switch (this->grouping) {
 			case GB_NONE: