Opened 16 years ago

Closed 16 years ago

Last modified 10 months ago

#8271 closed patch

Fix Read Overrun

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

Description

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 Changed 16 years ago by SF/quietust

I assume you meant "longer than 18 chars"

comment:2 Changed 16 years ago by SF/arm_in

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 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

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 Changed 16 years ago by fingolfin

Owner: set to fingolfin
Status: newclosed

comment:6 Changed 10 months ago by digitall

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