Opened 16 years ago

Closed 16 years ago

Last modified 13 months ago

#8271 closed patch

Fix Read Overrun

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


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, 16 years ago

I assume you meant "longer than 18 chars"

comment:2 by SF/arm_in, 16 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, 16 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, 16 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, 16 years ago

Owner: set to fingolfin
Status: newclosed

comment:6 by digitall, 13 months ago

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