From ee8def69aea629d23e3aca5dba5e758f1ff5d7db Mon Sep 17 00:00:00 2001
From: Daljit <daljit7991@gmail.com>
Date: Wed, 21 Aug 2024 11:43:32 +0100
Subject: [PATCH] Fix invalid memory access when reading colour maps

In 62861585ac9dedfdc64d2879124ab392ac4736a4, as part of #2911,
ColourMap::maps was converted from a null-terminated C array to a
std::vector. However, the loops iterating over this list weren't updated
and were still relying on null-termination causing an invalid memory
access. This also resulted mrview crashing on MacOS.
---
 src/colourmap.h                     | 33 ++++++++---------------------
 src/gui/mrview/colourmap_button.cpp | 12 +++++------
 src/gui/mrview/gui_image.cpp        |  9 +++++---
 src/gui/mrview/tool/fixel/fixel.cpp |  4 ++--
 4 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/src/colourmap.h b/src/colourmap.h
index 897806df0a..f3615c61e6 100644
--- a/src/colourmap.h
+++ b/src/colourmap.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include "exception.h"
 #include "types.h"
 #include <functional>
 
@@ -51,38 +52,22 @@ class Entry {
 
 extern const std::vector<Entry> maps;
 
-inline size_t num() {
-  size_t n = 0;
-  while (maps[n].name)
-    ++n;
-  return n;
-}
+inline size_t num() { return maps.size(); }
 
 inline size_t num_scalar() {
-  size_t n = 0, i = 0;
-  while (maps[i].name) {
-    if (!maps[i].special)
-      ++n;
-    ++i;
-  }
-  return n;
+  return std::count_if(maps.begin(), maps.end(), [](const Entry &map) { return map.special; });
 }
 
 inline size_t index(const std::string &name) {
-  size_t n = 0;
-  while (maps[n].name != name)
-    ++n;
-  return n;
+  auto it = std::find_if(maps.begin(), maps.end(), [&name](const Entry &map) { return map.name == name; });
+
+  if (it == maps.end())
+    throw MR::Exception("Colour map \"" + name + "\" not found");
+  return std::distance(maps.begin(), it);
 }
 
 inline size_t num_special() {
-  size_t n = 0, i = 0;
-  while (maps[i].name) {
-    if (maps[i].special)
-      ++n;
-    ++i;
-  }
-  return n;
+  return std::count_if(maps.begin(), maps.end(), [](const Entry &map) { return map.special; });
 }
 
 } // namespace MR::ColourMap
diff --git a/src/gui/mrview/colourmap_button.cpp b/src/gui/mrview/colourmap_button.cpp
index bbe4c911ca..b6d331c349 100644
--- a/src/gui/mrview/colourmap_button.cpp
+++ b/src/gui/mrview/colourmap_button.cpp
@@ -41,9 +41,9 @@ void ColourMapButton::init_core_menu_items(bool create_shortcuts) {
   core_colourmaps_actions->setExclusive(true);
 
   size_t n = 0;
-  for (size_t i = 0; ColourMap::maps[i].name; ++i) {
-    if (!ColourMap::maps[i].special && !ColourMap::maps[i].is_colour) {
-      QAction *action = new QAction(ColourMap::maps[i].name, this);
+  for (const auto &map : ColourMap::maps) {
+    if (!map.special && !map.is_colour) {
+      QAction *action = new QAction(map.name, this);
       action->setCheckable(true);
       core_colourmaps_actions->addAction(action);
 
@@ -82,9 +82,9 @@ void ColourMapButton::init_custom_colour_menu_items() {
 
 void ColourMapButton::init_special_colour_menu_items(bool create_shortcuts) {
   size_t n = colourmap_actions.size();
-  for (size_t i = 0; ColourMap::maps[i].name; ++i) {
-    if (ColourMap::maps[i].special) {
-      QAction *action = new QAction(ColourMap::maps[i].name, this);
+  for (const auto &map : ColourMap::maps) {
+    if (map.special) {
+      QAction *action = new QAction(map.name, this);
       action->setCheckable(true);
       core_colourmaps_actions->addAction(action);
 
diff --git a/src/gui/mrview/gui_image.cpp b/src/gui/mrview/gui_image.cpp
index a7c26efd77..4673c3953f 100644
--- a/src/gui/mrview/gui_image.cpp
+++ b/src/gui/mrview/gui_image.cpp
@@ -115,9 +115,12 @@ size_t Image::guess_colourmap() const {
     if (header().size(3) == 3)
       map = "RGB";
   }
-  for (size_t n = 0; ColourMap::maps[n].name; ++n)
-    if (ColourMap::maps[n].name == map)
-      return n;
+  auto it = std::find_if(
+      ColourMap::maps.begin(), ColourMap::maps.end(), [&map](const auto &entry) { return entry.name == map; });
+  if (it != ColourMap::maps.end()) {
+    return std::distance(ColourMap::maps.begin(), it);
+  }
+
   return 0;
 }
 
diff --git a/src/gui/mrview/tool/fixel/fixel.cpp b/src/gui/mrview/tool/fixel/fixel.cpp
index d597b7db6e..aed8a5b189 100644
--- a/src/gui/mrview/tool/fixel/fixel.cpp
+++ b/src/gui/mrview/tool/fixel/fixel.cpp
@@ -415,8 +415,8 @@ void Fixel::update_gui_colour_controls(bool reload_colour_types) {
   // how many menu elements were actually created by ColourMap::create_menu()
   static size_t colourmap_count = 0;
   if (!colourmap_count) {
-    for (size_t i = 0; ColourMap::maps[i].name; ++i) {
-      if (!ColourMap::maps[i].special)
+    for (const auto &map : ColourMap::maps) {
+      if (!map.special)
         ++colourmap_count;
     }
   }