[opensource-dev] Miscellaneous bad code in llviewerobject.cpp

John Nagle nagle at animats.com
Fri Sep 21 16:51:14 PDT 2018


    I've been looking for the "half-unsit" region crossing bug,
adding debug code to llviewerobject.  Something is going wrong
in reparenting, but I haven’t been able to isolate it yet.

    Looking in there, I've found some code that's wrong, but
probably not causing trouble.

- Two possible buffer overflows. Reported to LL, being treated as
a security issue.

- Unchecked downcasts on polymorphic types.  Those are
risky. There's at least one place where an unexpected message
from the sim will cause an invalid downcast. That's
been reported to LL. Newer fixes use dynamic_cast, but
some of the older ones don't.  There's one place
where there's an unchecked downcast followed by a
test for null. That's undefined behavior on a class with
multiple inheritance. It seems to work because it's a
downcast on the first parent class, but that's an
accident of the C++ implementation.

- Can this test, around line 1650, ever return true?

// BUG: this is a bad assumption once border crossing is alowed
     if (  (parent_id == cur_parentp->mLocalID)
	&&(update_type == OUT_TERSE_IMPROVED))

If update type is OUT_TERSE_IMPROVED, there can't be a parent id,
I think. Only full updates have parent info. So that path should
never be taken. Should the "&&" be a "||"?

None of this is the region crossing bug; it's just stuff I am
finding as I look for that.

(Does anybody still read this list?)



More information about the opensource-dev mailing list