[haiku-bugs] Re: [Haiku] #12340: FlattenPictureTest kills app_server (Regression)

  • From: "mmlr" <trac@xxxxxxxxxxxx>
  • Date: Mon, 31 Aug 2015 09:32:58 -0000

#12340: FlattenPictureTest kills app_server (Regression)
----------------------------------+----------------------------
Reporter: jackburton | Owner: axeld
Type: bug | Status: new
Priority: critical | Milestone: Unscheduled
Component: Servers/app_server | Version: R1/Development
Resolution: | Keywords:
Blocked By: | Blocking:
Has a Patch: 0 | Platform: All
----------------------------------+----------------------------

Comment (by mmlr):

The code of PicturePlayer::Play() looks very dangerous in general. It uses
signed integers for reading the size, but doesn't check for sizes < 0
anywhere, making it possible to go backwards in the buffer. It also does
just cast the data to the needed type without checking if the size
actually matches the type, which can easily lead to buffer overflows
(accidental or otherwise, since the size field comes from the supplied
buffer it should not be blindly trusted).

Code style wise it also uses magic numbers for the field header size
everywhere instead of making it a packed struct and using sizeof().

In this concrete case the debugger is called due to bogus values for the
opcode and size coming from the buffer (the values are random, so it looks
like uninitialized memory is used). It is triggered when playing the
unarchived picture of the no-op test, while the original picture (the one
not archived and then unarchived) plays fine, indicating that there's a
problem with archiving/unarchiving an empty BPicture.

I'll go ahead and rework PicturePlayer::Play() later today if noone else
feels inclined to take a look.

--
Ticket URL: <https://dev.haiku-os.org/ticket/12340#comment:3>
Haiku <https://dev.haiku-os.org>
Haiku - the operating system.

Other related posts: