On Sat, 8 May 2021 at 12:27, D. Hugh Redelmeier <[email protected]> wrote:
> | commit 9945236619b17fa13dfd1cfbe60359dcbf3fcd21 > | Author: D. Hugh Redelmeier <[email protected]> > | Date: Fri May 7 23:58:52 2021 -0400 > | > | pluto: packet.c: add consistency check to pbs_in_struct > | > | pbs_in_struct now requires that if and only if the structure has a > | length field then the obj_pbs isn't NULL. > | > | There are a few places where this part was intentionally ignored. > | This required adding a dummy pbs variable ("ignored"). > > Structs, as understood by packet.h, sometimes have variable parts > after the fixed fields. > > Here's an extract from the ancient comments at the head of > pbs_in_struct. The last parameter of pbs_in_struct is > struct pbs_in *obj_pbs" > The phrase "is supplied" means "is not NULL". > > The old behavior was that a NULL implied that the nested struct, if any, could be ignored. I'd call that obvious behaviour. (passing a pbs when not needed was benign). Now, if this is done wrong, and on a rarely executed code path, we've got a ticking time bomb. It's no longer obvious from a simple visual check of the call that passing in NULL or not isn't going to explode. > * If obj_pbs is supplied, a new pb_stream is created for the > * variable part of the structure (this depends on their > * being one length field in the structure). The cursor of this > * new PBS is set to after the parsed part of the struct. > > I was prompted to add this because it would have caught a mistake we > made but never committed. It has not caught any actual bugs in our > code. The more checking, the more confidence we can have in our code. > Would it have crashed pluto?
_______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
