Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 17fa5c7c authored by Tao Bao's avatar Tao Bao
Browse files

ui: Manage menu_ with std::vector. 

Prior to this CL, menu_ is allocated with a fixed length of text_rows_.
However, because we support scrollable menu in wear_ui, there might be
more menu entries than text_rows_, which would lead to out-of-bounds
array access. This CL addresses the issue by switching to std::vector.

Bug: 65416558
Test: Run 'View recovery logs' on angler.
Test: Set large margin height that leaves text_rows less than 21. Then
      run 'View recovery logs' with 21 menu entries.
Change-Id: I5d4e3a0a097039e1104eda7d494c6269053dc894
(cherry picked from commit e15d7a5104978cd8399501636aec0df9c1a4823c)
parent c16d2225

screen_ui.cpp

+7 −9
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ ScreenRecoveryUI::ScreenRecoveryUI() 
      text_top_(0),

      show_text(false),

      show_text_ever(false),

      menu_(nullptr),

      menu_headers_(nullptr),

      show_menu(false),

      menu_items(0),

      menu_sel(0),
@@ -356,10 +356,10 @@ void ScreenRecoveryUI::draw_screen_locked() { 
        DrawHighlightBar(0, y - 2, gr_fb_width(), char_height_ + 4);

        // Bold white text for the selected item.

        SetColor(MENU_SEL_FG);

        y += DrawTextLine(x, y, menu_[i], true);

        y += DrawTextLine(x, y, menu_[i].c_str(), true);

        SetColor(MENU);

      } else {

        y += DrawTextLine(x, y, menu_[i], false);

        y += DrawTextLine(x, y, menu_[i].c_str(), false);

      }

    }

    y += DrawHorizontalRule(y);
@@ -508,7 +508,6 @@ bool ScreenRecoveryUI::Init(const std::string& locale) { 


  text_ = Alloc2d(text_rows_, text_cols_ + 1);

  file_viewer_text_ = Alloc2d(text_rows_, text_cols_ + 1);

  menu_ = Alloc2d(text_rows_, text_cols_ + 1);



  text_col_ = text_row_ = 0;

  text_top_ = 1;
@@ -771,12 +770,11 @@ void ScreenRecoveryUI::StartMenu(const char* const* headers, const char* const* 
  pthread_mutex_lock(&updateMutex);

  if (text_rows_ > 0 && text_cols_ > 0) {

    menu_headers_ = headers;

    size_t i = 0;

    for (; i < text_rows_ && items[i] != nullptr; ++i) {

      strncpy(menu_[i], items[i], text_cols_ - 1);

      menu_[i][text_cols_ - 1] = '\0';

    menu_.clear();

    for (size_t i = 0; i < text_rows_ && items[i] != nullptr; ++i) {

      menu_.emplace_back(std::string(items[i], strnlen(items[i], text_cols_ - 1)));

    }

    menu_items = i;

    menu_items = static_cast<int>(menu_.size());

    show_menu = true;

    menu_sel = initial_selection;

    update_screen_locked();

screen_ui.h

+2 −1
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include <stdio.h>



#include <string>

#include <vector>



#include "ui.h"
@@ -127,7 +128,7 @@ class ScreenRecoveryUI : public RecoveryUI { 
  bool show_text;

  bool show_text_ever;  // has show_text ever been true?



  char** menu_;

  std::vector<std::string> menu_;

  const char* const* menu_headers_;

  bool show_menu;

  int menu_items, menu_sel;

wear_ui.cpp

+6 −7
Original line number Diff line number Diff line
@@ -154,11 +154,11 @@ void WearRecoveryUI::draw_screen_locked() { 
          // white text of selected item

          SetColor(MENU_SEL_FG);

          if (menu_[i][0]) {

            gr_text(gr_sys_font(), x + 4, y, menu_[i], 1);

            gr_text(gr_sys_font(), x + 4, y, menu_[i].c_str(), 1);

          }

          SetColor(MENU);

        } else if (menu_[i][0]) {

          gr_text(gr_sys_font(), x + 4, y, menu_[i], 0);

          gr_text(gr_sys_font(), x + 4, y, menu_[i].c_str(), 0);

        }

        y += char_height_ + 4;

      }
@@ -255,17 +255,16 @@ void WearRecoveryUI::StartMenu(const char* const* headers, const char* const* it 
  pthread_mutex_lock(&updateMutex);

  if (text_rows_ > 0 && text_cols_ > 0) {

    menu_headers_ = headers;

    size_t i = 0;

    menu_.clear();

    // "i < text_rows_" is removed from the loop termination condition,

    // which is different from the one in ScreenRecoveryUI::StartMenu().

    // Because WearRecoveryUI supports scrollable menu, it's fine to have

    // more entries than text_rows_. The menu may be truncated otherwise.

    // Bug: 23752519

    for (; items[i] != nullptr; i++) {

      strncpy(menu_[i], items[i], text_cols_ - 1);

      menu_[i][text_cols_ - 1] = '\0';

    for (size_t i = 0; items[i] != nullptr; i++) {

      menu_.emplace_back(std::string(items[i], strnlen(items[i], text_cols_ - 1)));

    }

    menu_items = i;

    menu_items = static_cast<int>(menu_.size());

    show_menu = true;

    menu_sel = initial_selection;

    menu_start = 0;