mash/mash-1/codec/p64 p64.cc

[Jose Maria Gonzalez <chema@bmrc.berkeley.edu>]

Update of /usr/mash/src/repository/mash/mash-1/codec/p64
In directory gumby.cs.berkeley.edu:/tmp/cvs-serv22927

Modified Files:
	p64.cc 
Log Message:
Problem:
Fixed nasty bug on the "which blocks have just changed" 
table (called marks_ in the P64decode world, rvts_ in the 
Decoder and H261Decoder ones, and crvec_ in the Renderer 
one).

Explanation:
This structure is a (width_in_number_of_8x8_blocks *
height_in_number_of_8x8_blocks) buffer where for each frame
that is decoded the decoder writes up which blocks have just
changed. This way the Rendererers may have a much better time
trying to display the new frame.

The problem consisted of the following: when a remote source
changes the size of the video it is transmitting, you get an
RTP packet with a picture header that states such size change.
This will fire P64Decoder::init() in the decoder, which changes
P64Decoder frame dimensions and permits it creating a new buffer
for the frame data while deleting the old one.

The size-change for the rvts_ buffer, however, is fired by
H261Decoder::recv(), and only after the packet has been fully decoded.
As a part of the packet-decoding process, P64Decoder::decode_mb()
tries to register which new 8x8-blocks it has received in the
packet. P64Decoder::decode_mb() data is correct, but the marks_
buffer where it writes up such information was created for
different-size frames. Old buffer, new dimensions... If the
frame-size change is from QCIF to CIF we have a problem.

At least in Linux, the problem was not detected by a kernel
SIGSEGV when we tried to write further from marks_ limits, though.
Linux malloc.c permits us writing past a buffer if we own the next
buffer. Actually vic crashed because P64Decoder::decode_mb()
overruning meant writing in the next memory chunk (malloc_chunk
structure). When we deleted the old rvts_, the kernel tried to
rebuild the memory by consolidating the old block backward (adding it
to the previous chunk, which worked ok) and then forward (writing up
in the next chunk the size of the previous one). The second step
meant accessing to a far-away memory position, which indeed did
SIGSEGV.

This bug is actually pretty similar to Kaempf's 1999 LBL-traceroute
vulnerability, a well-known bug. It seems hard to exploit just by
sending RTP packets, though.

See http://161.53.42.3/~crv/security/bugs/mUNIXes/troute4.html for
more info.


Solution:
As it seems a design decision not to access to the H261Decoder
object from the P64 one, the way to avoid it happening is
voiding marks_ when the decoder dimensions change. This happens
in P64Decoder::init(), so we added a line to set the pointer to
zero. There is no danger of leaking memory because the Decoder
object has its own copy of the buffer address, so it will be able
to free it.


Plus:
  - P64Decoder::decode now returns the number of MacroBlocks
it encoded instead

  - I also took advantage of the debug to add some comments on
the H.261 picture header structure and its color-subsampling
scheme.