[Debian GNUstep maintainers] Bug#767847: Bug#767847: gworkspace: FTBFS on hurd-i386

[Ivan Vučica]

sri, 20. pro 2017. u 20:33 Yavor Doganov <yavor at gnu.org> napisao je:

> Hi,
>
> On Sun, Nov 02, 2014 at 11:24:11PM +0100, Svante Signell wrote:
> > Source: gworkspace
> > Version: 0.9.2-1
>
> > Currently gworkspace FTBFS on GNU/Hurd due to usage of PATH_MAX, which
> > is not defined. It did build previously, latest Hurd version is
> > 0.8.8-1.2+b1. The attached patch solves this by allocating the newpath
> > string in GWMetadata/gmds/gmds/gmds.m and
> > GWMetadata/gmds/mdextractor/mdextractor.m dynamically.
>
> Many thanks for the patch and sorry for the belated response.
> However, IMHO the patch is not completely correct:
>
> > --- gworkspace-0.9.2.orig/GWMetadata/gmds/gmds/gmds.m
> > +++ gworkspace-0.9.2/GWMetadata/gmds/gmds/gmds.m
> > -  char newpath[PATH_MAX] = "";
> > +  char *newpath = NULL;
> >    int i = newblen;
> >    int j;
> >
> > -  strncpy(newpath, (const char *)newbase, newblen);
> > -
> > +  newpath = malloc(newblen) + 1;
> > +  strncpy(newpath, (const char *)newbase, newblen + 1);
>
> newblen is not sufficient size for the allocated string.  If oldbase
> is "/foo", newbase is "/frob" and oldpath is "/foo/bar", newpath
> should become "/frob/bar", so there would be an invalid write in the
> loop below.  If newbase is "/frobnication" it gets worse.


Also malloc(...)+1 should be malloc(...+1). (You addressed this differently
in your patch but I can't see enough context on my phone to judge if this
or other stuff is right.)

Also strncpy() IIRC doesn’t add a nul terminator.

How about calloc() to also zero the memory out, or at least
newpath[newblen] = 0? (Or the equivalent in the new patch.)

Alternatively just use ObjC to manipulate strings of UTF8 text.



>
> >    for (j = oldblen; j < oldplen; j++) {
> >      newpath[i] = oldpath[j];
> >      i++;
>
> I plan to apply a slightly modified version of the patch (attached)
> that fixes the above problem, removes the unnecessary initial
> assignment and also frees the allocated string (these programs are
> daemons so memory leaks should be avoided).  Ideally, there should be
> a check for malloc failure as well but I'll leave that to upstream as
> I'm not sure what should be done -- silent return, abort, or let the
> program crash.
>
> OK?
> _______________________________________________
> Debian GNUstep maintainers mailing list
> pkg-GNUstep-maintainers at lists.alioth.debian.org
>
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-gnustep-maintainers

-- 
Sent from Gmail Mobile on iPad
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pkg-gnustep-maintainers/attachments/20171220/ba3431e1/attachment-0003.html>