From c0bdc14cea97f1826acf4459a2fb92fb1e5c7973 Mon Sep 17 00:00:00 2001 From: Marco Zehe Date: Thu, 7 Feb 2019 21:06:42 +0000 Subject: [PATCH] Bug 1525849 - Guard against 0 columns or out of bounds indexes for ARIA grid accessibles, r=Jamie When returning the column or row index of a given cell, guard against the column count being 0 or the given index being out of bounds of the current grid or table. The MSAA code already did this previously, but now the upper bounds check has been moved to the base classes and an additional guard for the column count been put in place so a division by 0 crash canot happen. A return value for RowIndexAt and ColIndexAt of -1 indicates an error condition. ATK will automatically deal with this, and the IA2 code has been adjusted to check for this and return an invalid argument error in such cases, too. Differential Revision: https://phabricator.services.mozilla.com/D18931 --HG-- extra : moz-landing-system : lando --- accessible/generic/TableAccessible.cpp | 31 +++++++++++++++++++ accessible/generic/TableAccessible.h | 20 ++++++------ accessible/windows/ia2/ia2AccessibleTable.cpp | 30 +++++++++++++----- 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/accessible/generic/TableAccessible.cpp b/accessible/generic/TableAccessible.cpp index 9f75cf4dc5e4..b5c2f3498651 100644 --- a/accessible/generic/TableAccessible.cpp +++ b/accessible/generic/TableAccessible.cpp @@ -263,3 +263,34 @@ Accessible* TableAccessible::CellInRowAt(Accessible* aRow, int32_t aColumn) { return cell; } + +int32_t TableAccessible::ColIndexAt(uint32_t aCellIdx) { + uint32_t colCount = ColCount(); + if (colCount < 1 || aCellIdx >= colCount * RowCount()) { + return -1; // Error: column count is 0 or index out of bounds. + } + + return aCellIdx % colCount; +} + +int32_t TableAccessible::RowIndexAt(uint32_t aCellIdx) { + uint32_t colCount = ColCount(); + if (colCount < 1 || aCellIdx >= colCount * RowCount()) { + return -1; // Error: column count is 0 or index out of bounds. + } + + return aCellIdx / colCount; +} + +void TableAccessible::RowAndColIndicesAt(uint32_t aCellIdx, int32_t* aRowIdx, + int32_t* aColIdx) { + uint32_t colCount = ColCount(); + if (colCount < 1 || aCellIdx >= colCount * RowCount()) { + *aRowIdx = -1; + *aColIdx = -1; + return; // Error: column count is 0 or index out of bounds. + } + + *aRowIdx = aCellIdx / colCount; + *aColIdx = aCellIdx % colCount; +} diff --git a/accessible/generic/TableAccessible.h b/accessible/generic/TableAccessible.h index f6eab954697f..8a6229cd7805 100644 --- a/accessible/generic/TableAccessible.h +++ b/accessible/generic/TableAccessible.h @@ -56,27 +56,25 @@ class TableAccessible { /** * Return the column index of the cell with the given index. + * This returns -1 if the column count is 0 or an invalid index is being + * passed in. */ - virtual int32_t ColIndexAt(uint32_t aCellIdx) { - return aCellIdx % ColCount(); - } + virtual int32_t ColIndexAt(uint32_t aCellIdx); /** * Return the row index of the cell with the given index. + * This returns -1 if the column count is 0 or an invalid index is being + * passed in. */ - virtual int32_t RowIndexAt(uint32_t aCellIdx) { - return aCellIdx / ColCount(); - } + virtual int32_t RowIndexAt(uint32_t aCellIdx); /** * Get the row and column indices for the cell at the given index. + * This returns -1 for both output parameters if the column count is 0 or an + * invalid index is being passed in. */ virtual void RowAndColIndicesAt(uint32_t aCellIdx, int32_t* aRowIdx, - int32_t* aColIdx) { - uint32_t colCount = ColCount(); - *aRowIdx = aCellIdx / colCount; - *aColIdx = aCellIdx % colCount; - } + int32_t* aColIdx); /** * Return the number of columns occupied by the cell at the given row and diff --git a/accessible/windows/ia2/ia2AccessibleTable.cpp b/accessible/windows/ia2/ia2AccessibleTable.cpp index 0a4344fa7343..7150c13da24b 100644 --- a/accessible/windows/ia2/ia2AccessibleTable.cpp +++ b/accessible/windows/ia2/ia2AccessibleTable.cpp @@ -137,11 +137,16 @@ ia2AccessibleTable::get_columnIndex(long aCellIdx, long* aColIdx) { *aColIdx = 0; if (!mTable) return CO_E_OBJNOTCONNECTED; - if (aCellIdx < 0 || static_cast(aCellIdx) >= - mTable->ColCount() * mTable->RowCount()) + if (aCellIdx < 0) { return E_INVALIDARG; + } - *aColIdx = mTable->ColIndexAt(aCellIdx); + long colIdx = mTable->ColIndexAt(aCellIdx); + if (colIdx == -1) { // Indicates an error. + return E_INVALIDARG; + } + + *aColIdx = colIdx; return S_OK; } @@ -246,11 +251,16 @@ ia2AccessibleTable::get_rowIndex(long aCellIdx, long* aRowIdx) { *aRowIdx = 0; if (!mTable) return CO_E_OBJNOTCONNECTED; - if (aCellIdx < 0 || static_cast(aCellIdx) >= - mTable->ColCount() * mTable->RowCount()) + if (aCellIdx < 0) { return E_INVALIDARG; + } - *aRowIdx = mTable->RowIndexAt(aCellIdx); + long rowIdx = mTable->RowIndexAt(aCellIdx); + if (rowIdx == -1) { // Indicates an error. + return E_INVALIDARG; + } + + *aRowIdx = rowIdx; return S_OK; } @@ -406,12 +416,16 @@ ia2AccessibleTable::get_rowColumnExtentsAtIndex(long aCellIdx, long* aRowIdx, *aIsSelected = false; if (!mTable) return CO_E_OBJNOTCONNECTED; - if (aCellIdx < 0 || static_cast(aCellIdx) >= - mTable->ColCount() * mTable->RowCount()) + if (aCellIdx < 0) { return E_INVALIDARG; + } int32_t colIdx = 0, rowIdx = 0; mTable->RowAndColIndicesAt(aCellIdx, &rowIdx, &colIdx); + if (rowIdx == -1 || colIdx == -1) { // Indicates an error. + return E_INVALIDARG; + } + *aRowIdx = rowIdx; *aColIdx = colIdx; *aRowExtents = mTable->RowExtentAt(rowIdx, colIdx);