nethack-gwaa-kiwi/nethack
1
0
Fork 0
Code Pull Requests Activity

Amiga: defensive NULL/bounds guards in menus and window creation

Browse Source 
Guard the gd lookup in DoMenuScroll's GADGETUP/MOUSEMOVE branches
so a window with no GadgetID==1 does not deref NULL; match the
existing guards in the keyboard-scroll branches.  In the keyboard
selector and MENU_UNSELECT_ALL paths, only mutate items with
canselect set so a non-selectable header cannot have its str
stomped.  Clamp MENU_LAST_PAGE topidx to >= 0.  Make find_menu_item
return NULL on negative idx instead of the head item.  Guard the
PROMPTFIRST data[] shuffle behind cury > 0.

In amii_destroy_nhwindow's NHW_OVER branch use cw->win with a NULL
guard instead of dereferencing amii_wins[WIN_OVER]->win blindly.
Range-check the type argument to amii_create_nhwindow.  Fix the
*argv_in[1] precedence bug so the -L/-l flag does not deref NULL
when it is the last argument.  Wrap AllocAslRequest result in a
NULL check before AslRequestTags/FreeAslRequest.

Defensively bounds-check the idx argument to DispCol.  Replace
the -25937 signed-int literal in clipwin's PropInfo with the
equivalent UWORD value 39599.  Simplify amii_start_menu's free
loop; switch DoMenuScroll's inventory title and Count display to
Snprintf, and stop passing countString to pline as a format.
This commit is contained in:
Ingo Paschke
2026-05-12 15:27:13 +02:00
parent 15e3973ac2
commit 252ca5bef7
4 changed files with 56 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sys/amiga/clipwin.c
+1 -1
View File
@@ -197,7 +197,7 @@ static struct Gadget ClipXSIZE = {
static struct PropInfo ClipClipYSIZESInfo = { static struct PropInfo ClipClipYSIZESInfo = {
AUTOKNOB + FREEHORIZ, /* PropInfo flags */ AUTOKNOB + FREEHORIZ, /* PropInfo flags */
-25937, -1, /* horizontal and vertical pot values */ 39599, -1, /* horizontal and vertical pot values */
10922, -1, /* horizontal and vertical body values */ 10922, -1, /* horizontal and vertical body values */
}; };
sys/amiga/winamenu.c
+23 -11
View File
@@ -32,8 +32,7 @@ amii_start_menu(winid window, unsigned long mbehavior UNUSED)
cw->data = NULL; cw->data = NULL;
} }
for (mip = cw->menu.items, i = 0; while ((mip = cw->menu.items) != NULL) {
(mip = cw->menu.items) && i < cw->menu.count; ++i) {
cw->menu.items = mip->next; cw->menu.items = mip->next;
free(mip); free(mip);
} }
@@ -149,11 +148,13 @@ amii_end_menu(winid window, const char *morestr)
cw->menu.last->next = cw->menu.items; cw->menu.last->next = cw->menu.items;
cw->menu.items = cw->menu.last; cw->menu.items = cw->menu.last;
cw->menu.last = mip; cw->menu.last = mip;
t = cw->data[cw->cury - 1]; if (cw->cury > 0) {
for (i = cw->cury - 1; i > 0; i--) { t = cw->data[cw->cury - 1];
cw->data[i] = cw->data[i - 1]; for (i = cw->cury - 1; i > 0; i--) {
cw->data[i] = cw->data[i - 1];
}
cw->data[0] = t;
} }
cw->data[0] = t;
#endif #endif
} }
@@ -190,6 +191,8 @@ amii_menu_item *
find_menu_item(struct amii_WinDesc *cw, int idx) find_menu_item(struct amii_WinDesc *cw, int idx)
{ {
amii_menu_item *mip; amii_menu_item *mip;
if (idx < 0)
return NULL;
for (mip = cw->menu.items; idx > 0 && mip; mip = mip->next) for (mip = cw->menu.items; idx > 0 && mip; mip = mip->next)
--idx; --idx;
@@ -336,7 +339,8 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
nw->Screen = HackScreen; nw->Screen = HackScreen;
if (win == WIN_INVEN) { if (win == WIN_INVEN) {
sprintf(title, "%s the %s's Inventory", svp.plname, svp.pl_character); Snprintf(title, sizeof title, "%s the %s's Inventory",
svp.plname, svp.pl_character);
nw->Title = title; nw->Title = title;
if (lastinvent.MaxX != 0) { if (lastinvent.MaxX != 0) {
nw->LeftEdge = lastinvent.MinX; nw->LeftEdge = lastinvent.MinX;
@@ -606,7 +610,8 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
if (how == PICK_ANY) { if (how == PICK_ANY) {
amip = cw->menu.items; amip = cw->menu.items;
while (amip) { while (amip) {
if (amip->selected) { if (amip->canselect && amip->selector
&& amip->selected) {
amip->selected = FALSE; amip->selected = FALSE;
amip->count = -1; amip->count = -1;
amip->str[SOFF + 2] = '-'; amip->str[SOFF + 2] = '-';
@@ -738,8 +743,9 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
} else { } else {
reset_counting = TRUE; reset_counting = TRUE;
} }
sprintf(countString, "Count: %d", count); Snprintf(countString, sizeof countString,
pline(countString); "Count: %ld", count);
pline("%s", countString);
} }
} else if (code == CTRL('D') || code == CTRL('U') } else if (code == CTRL('D') || code == CTRL('U')
|| code == MENU_NEXT_PAGE || code == MENU_NEXT_PAGE
@@ -761,7 +767,7 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
if (code == MENU_FIRST_PAGE) { if (code == MENU_FIRST_PAGE) {
topidx = 0; topidx = 0;
} else if (code == MENU_LAST_PAGE) { } else if (code == MENU_LAST_PAGE) {
topidx = cw->maxrow - wheight; topidx = max(0, cw->maxrow - wheight);
} else } else
for (i = 0; i < endcnt; ++i) { for (i = 0; i < endcnt; ++i) {
if (code == CTRL('D') || code == MENU_NEXT_PAGE) { if (code == CTRL('D') || code == MENU_NEXT_PAGE) {
@@ -851,6 +857,8 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
} else { } else {
int selected = FALSE; int selected = FALSE;
for (amip = cw->menu.items; amip; amip = amip->next) { for (amip = cw->menu.items; amip; amip = amip->next) {
if (!amip->canselect)
continue;
if (amip->selector == code) { if (amip->selector == code) {
if (how == PICK_ONE) if (how == PICK_ONE)
aredone = 1; aredone = 1;
@@ -905,6 +913,8 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
aredone = 1; aredone = 1;
for (gd = w->FirstGadget; gd && gd->GadgetID != 1;) for (gd = w->FirstGadget; gd && gd->GadgetID != 1;)
gd = gd->NextGadget; gd = gd->NextGadget;
if (!gd)
break;
pip = (struct PropInfo *) gd->SpecialInfo; pip = (struct PropInfo *) gd->SpecialInfo;
totalvis = CountLines(win); totalvis = CountLines(win);
@@ -917,6 +927,8 @@ DoMenuScroll(int win, int blocking, int how, menu_item **retmip)
case MOUSEMOVE: case MOUSEMOVE:
for (gd = w->FirstGadget; gd && gd->GadgetID != 1;) for (gd = w->FirstGadget; gd && gd->GadgetID != 1;)
gd = gd->NextGadget; gd = gd->NextGadget;
if (!gd)
break;
pip = (struct PropInfo *) gd->SpecialInfo; pip = (struct PropInfo *) gd->SpecialInfo;
totalvis = CountLines(win); totalvis = CountLines(win);
sys/amiga/winfuncs.c
+29 -22
View File
@@ -164,23 +164,23 @@ amii_destroy_nhwindow(winid win) /* just hide */
WIN_OVER = WIN_ERR; WIN_OVER = WIN_ERR;
} }
} else if (cw->type == NHW_OVER) { } else if (cw->type == NHW_OVER) {
struct Window *w = amii_wins[WIN_OVER]->win; struct Window *w = cw->win;
amii_oldover.MinX = w->LeftEdge; if (w) {
amii_oldover.MinY = w->TopEdge; amii_oldover.MinX = w->LeftEdge;
amii_oldover.MaxX = w->Width; amii_oldover.MinY = w->TopEdge;
amii_oldover.MaxY = w->Height; amii_oldover.MaxX = w->Width;
amii_oldover.MaxY = w->Height;
if (WIN_MESSAGE != WIN_ERR && amii_wins[WIN_MESSAGE]) { if (WIN_MESSAGE != WIN_ERR && amii_wins[WIN_MESSAGE]
w = amii_wins[WIN_MESSAGE]->win; && (w = amii_wins[WIN_MESSAGE]->win) != NULL) {
amii_oldmsg.MinX = w->LeftEdge; amii_oldmsg.MinX = w->LeftEdge;
amii_oldmsg.MinY = w->TopEdge; amii_oldmsg.MinY = w->TopEdge;
amii_oldmsg.MaxX = w->Width; amii_oldmsg.MaxX = w->Width;
amii_oldmsg.MaxY = w->Height; amii_oldmsg.MaxY = w->Height;
SizeWindow(amii_wins[WIN_MESSAGE]->win, SizeWindow(w,
(amiIDisplay->xpix (amiIDisplay->xpix - w->LeftEdge) - w->Width,
- amii_wins[WIN_MESSAGE]->win->LeftEdge) 0);
- amii_wins[WIN_MESSAGE]->win->Width, }
0);
} }
} }
} }
@@ -359,6 +359,9 @@ amii_create_nhwindow(int type)
panic("no memory for msg port"); panic("no memory for msg port");
} }
if (type < 0 || type > NHW_OVER)
panic("bad type %d in create_nhwindow", type);
nw = &new_wins[type].newwin; nw = &new_wins[type].newwin;
nw->Width = amiIDisplay->xpix; nw->Width = amiIDisplay->xpix;
nw->Screen = HackScreen; nw->Screen = HackScreen;
@@ -915,7 +918,7 @@ amii_init_nhwindows(int *argcp, char **argv)
for (t = 1; t <= lclargc; t++) { for (t = 1; t <= lclargc; t++) {
if (!strcmp("-L", *argv_in) || !strcmp("-l", *argv_in)) { if (!strcmp("-L", *argv_in) || !strcmp("-l", *argv_in)) {
bigscreen = (*argv_in[1] == 'l') ? -1 : 1; bigscreen = ((*argv_in)[1] == 'l') ? -1 : 1;
/* and eat the flag */ /* and eat the flag */
(*argcp)--; (*argcp)--;
} else { } else {
@@ -1123,12 +1126,16 @@ amii_init_nhwindows(int *argcp, char **argv)
SM_FilterHook.h_Data = 0; SM_FilterHook.h_Data = 0;
SM_FilterHook.h_SubEntry = 0; SM_FilterHook.h_SubEntry = 0;
SMR = AllocAslRequest(ASL_ScreenModeRequest, NULL); SMR = AllocAslRequest(ASL_ScreenModeRequest, NULL);
if (AslRequestTags(SMR, ASLSM_FilterFunc, (ULONG) &SM_FilterHook, if (SMR) {
TAG_END)) if (AslRequestTags(SMR, ASLSM_FilterFunc,
amii_scrnmode = SMR->sm_DisplayID; (ULONG) &SM_FilterHook, TAG_END))
else amii_scrnmode = SMR->sm_DisplayID;
else
amii_scrnmode = 0;
FreeAslRequest(SMR);
} else {
amii_scrnmode = 0; amii_scrnmode = 0;
FreeAslRequest(SMR); }
} }
if (forcenobig == 0) { if (forcenobig == 0) {
sys/amiga/winreq.c
+3
View File
@@ -704,6 +704,9 @@ DispCol(struct Window *w, int idx, UWORD *colors)
char buf[50]; char buf[50];
char *colname, *defval; char *colname, *defval;
if (idx < 0 || idx >= amii_numcolors)
return;
if (WINVERS_AMIV) { if (WINVERS_AMIV) {
colname = amiv_colnames[idx].name; colname = amiv_colnames[idx].name;
defval = amiv_colnames[idx].defval; defval = amiv_colnames[idx].defval;