formats/cassimg.cpp: Fixed bad image crash in tap format (MT8952)

[holub]

I hope this time the fix is more solid and non-destructive

[holub]

Alternatively I can change signature for all existing fill_wave

[holub]

I'm waiting for consensus. Have this ^^^ implemented locally or can easily refactor to loading error without crash if you insist.

[cuavas]

Can you explain what the issue is, how it happens, and what this change is supposed to do to deal with it? Alternatively, would it be better to just reject these bad tape images? Trying to “fix” or gracefully handle bad input has been a constant source of bugs over the years, which is why software is increasingly moving to an approach of rejecting bad input outright.

[ajrhacker]

software is increasingly moving to an approach of rejecting bad input outright

Has MAME been doing anything specific in this direction, or is it just a trend witnessed elsewhere?

[holub]

It's pretty much explained in the deleted comment.
When we calculate output size initially it's based on the headers in the input which can request more data than actual image size.
Wave production uses on same input but now without "knowing" how much size is available.

Rejecting this images is the possibility, but with this change we are in better shape, as we can try to convert as much as we can and potentially that will (and proven "is") be enough.

[cuavas]

software is increasingly moving to an approach of rejecting bad input outright

Has MAME been doing anything specific in this direction, or is it just a trend witnessed elsewhere?

Yes, MAME is a lot better at rejecting rather than crashing on thing like bad ZIP, PNG and ICO files. There’s also more error checking in cartridge loading, etc. to reject invalid input.

But it’s a trend in software in general, as the realisation has set in that “be liberal in what you accept and strict in what you produce” has cause more problems than it solves. In particular, there’s inconsistency in how different software interprets the same invalid input, and all manner of bugs caused by the complexity of trying to do something useful with invalid input.

[holub]

Another though:
in theory switching to error at this point can make some existing dump wrong.
Scenario is: image has a header which requesting 1 byte more than needed, in the current state that's not enough to couse illegal access exception. So currently we can have working image but it will cause error after change to strict validation.

[holub]

Maybe we can osd warning about wrong image. Not sure if a lot of people cares about logs - that will reduce false bugrepors.

[mgarlanger]

What about updating the ones which are still using legacy cassette routines, to use the newer routines? I did this for the Heath H8/H88 file (although my PR is still waiting for a review - #13250). This removes the need to determine the size up front (the base routines resize as needed).