<div><div><div dir="auto"><br></div><br><div class="gmail_quote"><div dir="auto">sri, 20. pro 2017. u 20:33 Yavor Doganov <<a href="mailto:yavor@gnu.org" target="_blank">yavor@gnu.org</a>> napisao je:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
On Sun, Nov 02, 2014 at 11:24:11PM +0100, Svante Signell wrote:<br>
> Source: gworkspace<br>
> Version: 0.9.2-1<br>
<br>
> Currently gworkspace FTBFS on GNU/Hurd due to usage of PATH_MAX, which<br>
> is not defined. It did build previously, latest Hurd version is<br>
> 0.8.8-1.2+b1. The attached patch solves this by allocating the newpath<br>
> string in GWMetadata/gmds/gmds/gmds.m and<br>
> GWMetadata/gmds/mdextractor/mdextractor.m dynamically.<br>
<br>
Many thanks for the patch and sorry for the belated response.<br>
However, IMHO the patch is not completely correct:<br>
<br>
> --- gworkspace-0.9.2.orig/GWMetadata/gmds/gmds/gmds.m<br>
> +++ gworkspace-0.9.2/GWMetadata/gmds/gmds/gmds.m<br>
> -  char newpath[PATH_MAX] = "";<br>
> +  char *newpath = NULL;<br>
>    int i = newblen;<br>
>    int j;<br>
><br>
> -  strncpy(newpath, (const char *)newbase, newblen);<br>
> -<br>
> +  newpath = malloc(newblen) + 1;<br>
> +  strncpy(newpath, (const char *)newbase, newblen + 1);<br>
<br>
newblen is not sufficient size for the allocated string.  If oldbase<br>
is "/foo", newbase is "/frob" and oldpath is "/foo/bar", newpath<br>
should become "/frob/bar", so there would be an invalid write in the<br>
loop below.  If newbase is "/frobnication" it gets worse.</blockquote><div dir="auto"><br></div><div dir="auto"><div dir="auto">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.)<br></div><div dir="auto"><br></div><div dir="auto">Also strncpy() IIRC doesn’t add a nul terminator.</div><div dir="auto"><br></div><div dir="auto">How about calloc() to also zero the memory out, or at least newpath[newblen] = 0? (Or the equivalent in the new patch.)</div><div dir="auto"><br></div><div dir="auto">Alternatively just use ObjC to manipulate strings of UTF8 text.</div></div><div dir="auto"><br></div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
>    for (j = oldblen; j < oldplen; j++) {<br>
>      newpath[i] = oldpath[j];<br>
>      i++;<br>
<br>
I plan to apply a slightly modified version of the patch (attached)<br>
that fixes the above problem, removes the unnecessary initial<br>
assignment and also frees the allocated string (these programs are<br>
daemons so memory leaks should be avoided).  Ideally, there should be<br>
a check for malloc failure as well but I'll leave that to upstream as<br>
I'm not sure what should be done -- silent return, abort, or let the<br>
program crash.<br>
<br>
OK?<br>
_______________________________________________<br>
Debian GNUstep maintainers mailing list<br>
<a href="mailto:pkg-GNUstep-maintainers@lists.alioth.debian.org" target="_blank">pkg-GNUstep-maintainers@lists.alioth.debian.org</a><br>
<a href="http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-gnustep-maintainers" rel="noreferrer" target="_blank">http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-gnustep-maintainers</a></blockquote></div></div></div><div dir="ltr">-- <br></div><div class="gmail_signature" data-smartmail="gmail_signature">Sent from Gmail Mobile on iPad</div>