Opened 11 years ago

Closed 11 years ago

Last modified 17 months ago

#8959 closed patch

Fix crash when using BS1 without portuguese data

Reported by: criezy Owned by: eriktorbjorn
Priority: normal Component: Engine: Sword1
Keywords: Cc:
Game: Broken Sword 1

Description

Some BS1 versions do not have portuguese subtitles in the text cluster file. This causes a crash when setting the language of the game to portuguese in ScummVM. The attached patch solves the crash by permitting ResMan::resHandle(uint32 id) to return a null pointer (instead of whatever lies in memory at the out of bound read) and adding a number of tests to handle that properly in other functions (and also to avoid other out of bound reads and writes).

As is, the patch is not really complete. There is no crash but all the displayed subtitles are "Error: Text not found" (i.e. the error string in ObjectMan). This is fixed by the patch #2602772 that I submitted yesterday (which changes the error string to an empty string to fix bug #1977094). The patch is against SVN 0.14.0 from February 16. It probably conflicts with the patch #2602772 for the changes in objectman.cpp, but this changes are very small and can be done manually (or I can prepare a new patch if needed). Most changes in this patch are in file resman.cpp

In case you are interested, the crash occurs because the swordres.rif file list only six groups for the cluster 2 (i.e. 6 languages in the text.clu files) instead of seven (which is normal as there really are only six languages in the text.clu files for this version). Therefore the groups array for the cluster 2 in the _prj variable has only six elements, and there is a lot of out of bound reading in that array when trying to read info for the seventh group (in _prj.clu[cluster].grp[group]). This causes strange things, like trying to allocate x bytes, where x is supposed to be the data size for the group but in that case can be anything. Ultimately it crashes when allocating the memory for the group data and writing the pointer address in memory allocated for another cluster file (if it has not crashed before).

Ticket imported from: #2606844. Ticket imported from: patches/1064.

Attachments (1)

bs1_portuguese_crash.patch (5.8 KB ) - added by criezy 11 years ago.
Patch to avoid crash with BS1 that do not have portuguese subtitles

Download all attachments as: .zip

Change History (6)

by criezy, 11 years ago

Attachment: bs1_portuguese_crash.patch added

Patch to avoid crash with BS1 that do not have portuguese subtitles

comment:1 by fingolfin, 11 years ago

Owner: set to eriktorbjorn

comment:2 by fingolfin, 11 years ago

Torbjörn, does this look right to you?

comment:3 by eriktorbjorn, 11 years ago

Status: newclosed

comment:4 by eriktorbjorn, 11 years ago

I guess so. I'm not that familiar with the BS1 resource management, but as far as I can see the patch mainly adds sanity checking. I've committed it, with minor modifications to accommodate for the earlier subtitles patch.

comment:5 by digitall, 17 months ago

Component: Engine: Sword1
Game: Broken Sword 1
Note: See TracTickets for help on using tickets.