Opened 21 years ago

Closed 21 years ago

Last modified 6 years ago

#8271 closed patch

Fix Read Overrun

Reported by: SF/arm_in Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game:


In file "summ\verbs.cpp", function "void Scumm::redrawV2Inventory()", line 279

memcpy() causes a read overrun when the source string is shorter than 18 chars. This _might_ lead to a page fault on some systems. Better replace with something like that:

strncpy((char *) msg, (char *) _messagePtr, 18),

Ticket imported from: #790433. Ticket imported from: patches/376.

Change History (6)

comment:1 by SF/quietust, 21 years ago

I assume you meant "longer than 18 chars"

comment:2 by SF/arm_in, 21 years ago

No, "shorter". For example, be the source string "Ticket". Now memcpy() would always copy 18 chars where there are only 7 to get.

In addition, be the memory page boundary behind the source string. Now if you read the 8th char: *bang - page fault*

comment:3 by fingolfin, 21 years ago

Indeed, that memcpy is bogus. However, strncpy isn't portable so we can't use it. But writing our own, or just inserting an equivalenty 2-line loops is trivial.

comment:4 by fingolfin, 21 years ago

I was mixing strncpy up with snprintf. Of course it's portable (and even if not, i.e. if somebody gets into trouble because of it, we then can still replace it, if we want).

Fix applied, thanks.

comment:5 by fingolfin, 21 years ago

Owner: set to fingolfin
Status: newclosed

comment:6 by digitall, 6 years ago

Component: Engine: SCUMM
Note: See TracTickets for help on using tickets.