Skip to content

Teams editor stores an out-of-range BasePlayer::teamNumber into ColorButton unchecked #188

Description

@kylelutze

Follow-up from Giszmo's review of #175 (non-blocking note 4).

Problem

TeamsEditor::TeamsEditor (src/map/edit/MapEditDialog.cpp) seeds each row's colour button straight from the stored header:

color[i]->setSelectedColor(gameHeader.getBasePlayer(i).teamNumber);

ColorButton::setSelectedColor stores the value unchecked (libgag/include/GUIButton.h), and the button only holds mapHeader.getNumberOfTeams() colours. A map or game file whose BasePlayer::teamNumber is outside [0, numberOfTeams) therefore:

  • reads past the end of the colour vector in ColorButton::paint (v[selColor], libgag/src/GUIButton.cpp) as soon as the dialog draws;
  • is fed back unchanged by TeamsEditor::generateGameHeader, which uses color[i]->getSelectedColor() as the new teamNumber and as the index for setAllyTeamNumber, so pressing OK writes the malformed value back out.

#175 stopped the ally-team widget from throwing on the same malformed input, so the dialog now opens instead of crashing, which makes this path reachable.

Reachability

The editor's own add/remove team path rebuilds the header (MapEdit::regenerateGameHeader, src/map/edit/MapEditClicks.cpp), so this only fires for a file that is already out of range on disk. Hardening, same class as #175.

Suggested fix

Clamp on the way in, in the same spirit as allyTeamNumberToWidgetIndex (src/AllyTeamWidgetIndex.h): map an out-of-range teamNumber to a defined row (0, or the player's own index) before calling setSelectedColor. Alternatively have ColorButton::setSelectedColor reject or wrap values >= v.size() so every caller is covered. Either way, add a test/ CppUnit case.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions