Opened 17 years ago

Closed 16 years ago

Last modified 16 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 by SF/quietust, 17 years ago

I assume you meant "longer than 18 chars"

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

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