Hi Laurent, I'm looking at this and wondering why valgrind does not catch the error. Thanks for the help. I'll try to get a testable fix for you asap.
-Pieter On Mon, Aug 19, 2013 at 12:47 PM, Laurent Alebarde <[email protected]> wrote: > Sorry Pieter, I made a mistake. My test (below) does not pass. I have to dig > more. > > > Le 19/08/2013 12:24, Laurent Alebarde a écrit : > > Pieter, > > I am far from having analysed all the CurveZMQ library, but the following > change passes the tests : > > src/cusrc/curvezmq_codec.c - s_encrypt : > > Before : > size_t box_size = crypto_box_ZEROBYTES + size; > byte *plain = malloc (box_size); > byte *box = malloc (box_size); > ..... > memcpy (target, box + crypto_box_BOXZEROBYTES, size + > crypto_box_BOXZEROBYTES); > > After : > size_t plain_size = crypto_box_ZEROBYTES + size; > byte *plain = malloc (plain_size); > size_t box_size = crypto_box_BOXZEROBYTES + size; > byte *box = malloc (box_size); > ..... > memcpy (target, box + crypto_box_BOXZEROBYTES, size); > > > > Le 18/08/2013 10:07, Pieter Hintjens a écrit : > > Thanks for this analysis. I'll look at it when I have an hour or two; > it's too tricky to fix in passing. > > On Thu, Aug 15, 2013 at 12:30 PM, Laurent Alebarde <[email protected]> > wrote: > > Hi all, > > I am reviewing the curvezmq code with a debugger : > > Fact : > in curvezmq_codec.c (s_encrypt from s_produce_hello, line 288) : > memcpy (target, box + crypto_box_BOXZEROBYTES, size + > crypto_box_BOXZEROBYTES); > copies from (box + crypto_box_BOXZEROBYTES) (size + crypto_box_BOXZEROBYTES) > bytes > Debugger aborts (while the executable itself in a shell works well). > > Consequences : > 1) an amount of (crypto_box_BOXZEROBYTES) undefined bytes after the box > are copied to target, while the box is malloc-ed with a size of > (crypto_box_ZEROBYTES + size) > 2) the spec "target must be nonce + box" is not fullfilled since the > first crypto_box_BOXZEROBYTES bytes of the box are not copied to target (may > be ok since they are zeros) > > Possible solutions : > 1) change to : memcpy (target, box + crypto_box_BOXZEROBYTES, size); > and set appropriatly the remaining (crypto_box_BOXZEROBYTES) bytes > of target > 2) change to : memcpy (target, box, size + crypto_box_BOXZEROBYTES); > and adapt the size of target : hello_t.box becomes : bytes box[96] > 3) change to : memcpy (target, box + crypto_box_BOXZEROBYTES, size); > and adapt the size of target : hello_t.box becomes : bytes box[64] > > In addition : box should be malloc-ed with a size of > (crypto_box_BOXZEROBYTES + size) instead of (crypto_box_ZEROBYTES + size) > > Cheers, > > > Laurent. > > _______________________________________________ > zeromq-dev mailing list > [email protected] > http://lists.zeromq.org/mailman/listinfo/zeromq-dev > > _______________________________________________ > zeromq-dev mailing list > [email protected] > http://lists.zeromq.org/mailman/listinfo/zeromq-dev > > > > > _______________________________________________ > zeromq-dev mailing list > [email protected] > http://lists.zeromq.org/mailman/listinfo/zeromq-dev > > > > _______________________________________________ > zeromq-dev mailing list > [email protected] > http://lists.zeromq.org/mailman/listinfo/zeromq-dev > _______________________________________________ zeromq-dev mailing list [email protected] http://lists.zeromq.org/mailman/listinfo/zeromq-dev
