items: add BUGFIX for SpawnWitch

[mewmew]

The logic handling the list of items for sale by Adria is broken. In particular, the logic of SortWitch requires the list to end with an item having item type ITYPE_NONE.

From SortWitch:

    j = 3;
    while (witchitem[j + 1]._itype != ITYPE_NONE) {
        j++;
    }

However, this puts a limit (19) on the maximum number of items being generated for the witchitem array (of length 20). The problem is that SpawnWitch may generate 20 items if iCnt = 17, as made possible by the following statement.

    iCnt = random_(51, 8) + 10;

Since there are 3 items that are always generated (mana, full mana and town portal), the total number of items generated by SpawnWitch may be 20, which exceeds the limit (19), thus causing the logic of SortWitch to go haywire and results in a buffer overflow.

[mewmew]

Same issue in SpawnHealer in Multi Player games, causing buffer overflow in SortHealer.

[AJenbo]

Isn't that an issue with SortWitch?

[galaxyhaxz]

I don't think it causes an overflow like the bug you mentioned, but there is an additional bug with SortHealer. It begins at item index 2, which is correct for single games. However in multi he also has resurrect scrolls, so the index should begin at 3 there.

    if (gbMaxPlayers != 1) {
        GetItemAttrs(0, IDI_RESURRECT, 1);
        healitem[2] = item[0];
        healitem[2]._iCreateInfo = lvl;
        healitem[2]._iStatFlag = TRUE;
        srnd = 3;
    } else srnd = 2;

Edit: Since the hardcoded resurrect scrolls come before the random item indices, it will stay 3rd on the list. however if one were to move it down on the list, it could get mixed up with other items.

[galaxyhaxz]

I took a closer look at this, I don't think it's actually bugged in this way, but rather another. If you look here:

    nsi = random(8) + 10;
    for (i = 3; i < nsi; i++) {

The random index is 10-17. This means at most we would be scanning indices 3-17 of the witch item array, leaving the last 3 untouched. This is because this is bugged and should be:

for (i = 3; i < nsi + 3; i++)

In order to actually touch all 20 indices. Of course, as mentioned, if this bug were fixed it would cause the overflow so you'd want to cap it to 19 or fix it another way.

EDIT: same bug applies to healer, he starts at index 2 or 3 and only goes to at most 17, leaving the last two slots untouched.

[mewmew]

Closed this PR, as the bug had another cause, as detailed by Andi in https://github.com/diasurgical/devilution/pull/2255#issuecomment-925453606