From 4fe99d7f7f9333e39413969279e1ba0f29994a8b Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Fri, 10 Jan 2025 15:44:49 -0800 Subject: [PATCH] fix std:clamp usage We appear to consistently assume that clamp can be called with 0, size as interval, but that actually results in a possible out of bounds access at the upper end. Signed-off-by: Dirk Hohndel --- core/statistics.cpp | 4 ++-- mobile-widgets/statsmanager.cpp | 8 ++++---- stats/chartlistmodel.cpp | 2 +- stats/statsstate.cpp | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/statistics.cpp b/core/statistics.cpp index d2932d442..e779871bc 100644 --- a/core/statistics.cpp +++ b/core/statistics.cpp @@ -158,7 +158,7 @@ stats_summary calculate_stats_summary(bool selected_only) process_dive(*dp, out.stats_by_depth[0]); int d_idx = dp->maxdepth.mm / (STATS_DEPTH_BUCKET * 1000); - d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET); + d_idx = std::clamp(d_idx, 0, STATS_MAX_DEPTH / STATS_DEPTH_BUCKET - 1); process_dive(*dp, out.stats_by_depth[d_idx + 1]); out.stats_by_depth[d_idx + 1].selection_size++; @@ -167,7 +167,7 @@ stats_summary calculate_stats_summary(bool selected_only) process_dive(*dp, out.stats_by_temp[0]); int t_idx = ((int)mkelvin_to_C(dp->mintemp.mkelvin)) / STATS_TEMP_BUCKET; - t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET); + t_idx = std::clamp(t_idx, 0, STATS_MAX_TEMP / STATS_TEMP_BUCKET - 1); process_dive(*dp, out.stats_by_temp[t_idx + 1]); out.stats_by_temp[t_idx + 1].selection_size++; diff --git a/mobile-widgets/statsmanager.cpp b/mobile-widgets/statsmanager.cpp index c4cfec9a5..43e76764c 100644 --- a/mobile-widgets/statsmanager.cpp +++ b/mobile-widgets/statsmanager.cpp @@ -81,7 +81,7 @@ void StatsManager::var1Changed(int idx) { if (uiState.var1.variables.empty()) return; - idx = std::clamp(idx, 0, (int)uiState.var1.variables.size()); + idx = std::clamp(idx, 0, (int)uiState.var1.variables.size() - 1); state.var1Changed(uiState.var1.variables[idx].id); updateUi(); } @@ -96,7 +96,7 @@ void StatsManager::var2Changed(int idx) { if (uiState.var2.variables.empty()) return; - idx = std::clamp(idx, 0, (int)uiState.var2.variables.size()); + idx = std::clamp(idx, 0, (int)uiState.var2.variables.size() - 1); state.var2Changed(uiState.var2.variables[idx].id); updateUi(); } @@ -111,7 +111,7 @@ void StatsManager::var2OperationChanged(int idx) { if (uiState.operations2.variables.empty()) return; - idx = std::clamp(idx, 0, (int)uiState.operations2.variables.size()); + idx = std::clamp(idx, 0, (int)uiState.operations2.variables.size() - 1); state.var2OperationChanged(uiState.operations2.variables[idx].id); updateUi(); } @@ -120,7 +120,7 @@ void StatsManager::sortMode1Changed(int idx) { if (uiState.sortMode1.variables.empty()) return; - idx = std::clamp(idx, 0, (int)uiState.sortMode1.variables.size()); + idx = std::clamp(idx, 0, (int)uiState.sortMode1.variables.size() - 1); state.sortMode1Changed(uiState.sortMode1.variables[idx].id); updateUi(); } diff --git a/stats/chartlistmodel.cpp b/stats/chartlistmodel.cpp index 88fa64edc..bb59e08e7 100644 --- a/stats/chartlistmodel.cpp +++ b/stats/chartlistmodel.cpp @@ -41,7 +41,7 @@ void ChartListModel::initIcon(ChartSubType type, const char *name, int iconSize) const QPixmap &ChartListModel::getIcon(ChartSubType type, bool warning) const { - int idx = std::clamp((int)type, 0, (int)ChartSubType::Count); + int idx = std::clamp((int)type, 0, (int)ChartSubType::Count - 1); return warning ? subTypeIcons[idx].warning : subTypeIcons[idx].normal; } diff --git a/stats/statsstate.cpp b/stats/statsstate.cpp index 34923a310..ebdea96d7 100644 --- a/stats/statsstate.cpp +++ b/stats/statsstate.cpp @@ -256,7 +256,7 @@ static ChartValidity chartValidity(const ChartTypeDesc &desc, if ((operation != StatsOperation::Invalid) != desc.var2HasOperations) return ChartValidity::Invalid; - return valid1 == ChartValidity::Undesired || valid2 == ChartValidity::Undesired ? + return valid1 == ChartValidity::Undesired || valid2 == ChartValidity::Undesired ? ChartValidity::Undesired : ChartValidity::Good; } @@ -454,7 +454,7 @@ static const StatsBinner *idxToBinner(const StatsVariable *v, int idx, bool seco void StatsState::var1Changed(int id) { - var1 = stats_variables[std::clamp(id, 0, (int)stats_variables.size())]; + var1 = stats_variables[std::clamp(id, 0, (int)stats_variables.size() - 1)]; validate(true); } @@ -478,7 +478,7 @@ void StatsState::var2Changed(int id) { // The "none" variable is represented by a nullptr var2 = id == none_idx ? nullptr - : stats_variables[std::clamp(id, 0, (int)stats_variables.size())]; + : stats_variables[std::clamp(id, 0, (int)stats_variables.size() - 1)]; validate(true); }