Bug#386197: dpkg: strtol checks don't fail when given a null string
On Mon, 23 Mar 2009, Guillem Jover wrote: > > You added an unwanted "*" in the check. I'll commit it soon after tests. > > Before closing this bug, I'd like to get all other instances of strto* > in the source tree reviewed, for this kind of missing checks or other > flacky usage. Fortunately there aren't many such calls: dpkg-deb/extract.c: r = strtol(lintbuf, &endp, 10); The content of lintbuf is copied from a set of non-null bytes. And the length of data copied is never null. dpkg-split/queue.c: pq->info.maxpartlen= strtol(p,&q,16); if (q==p || *q++ != '.') return 0; dpkg-split/queue.c: p=q; pq->info.thispartn= (int)strtol(p,&q,16); if (q==p || *q++ != '.') return 0; dpkg-split/queue.c: p=q; pq->info.maxpartn= (int)strtol(p,&q,16); if (q==p || *q) return 0; The check includes q==p so it's fine. dpkg-split/main.c: newpartsize= strtol(value,&endp,10); dpkg-split/info.c: r= strtoul(value,&endp,10); Those two should be fixed as well. lib/showpkg.c: w=strtol(ws+1,&endptr,0); This one is fine. lib/parsehelp.c:epoch= strtoul(string,&eepochcolon,10); This one is ok if we want to accept that ":1" == "0:1" as version number. src/filesdb.c: fso->uid=strtol(thisline + 1, &endptr, 10); src/filesdb.c: fso->gid=strtol(thisline + 1, &endptr, 10); src/filesdb.c:fso->mode=strtol(thisline, &endptr, 8); Those have to be fixed as well. It's particularly bad as what would look like to a comment for any admin is in fact interpreted like a uid==0 by the code. src/main.c: f_debug= strtoul(value,&endp,8); src/main.c: v= strtoul(value,&ep,0); src/main.c: v= strtoul(value,&ep,0); Must be fixed too. src/main.c: if ((infd= strtol(pipein, (char **)NULL, 10)) == -1) Clearly wrong too. utils/start-stop-daemon.c: ul = strtoul(string, &ep, 10); utils/start-stop-daemon.c: *value_r = strtoul(string, NULL, 0); One is ok, the other not. I'll commit fixes for all these later. Cheers, -- Raphaël Hertzog Contribuez à Debian et gagnez un cahier de l'admin Debian Lenny : http://www.ouaza.com/wp/2009/03/02/contribuer-a-debian-gagner-un-livre/ -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#386197: dpkg: strtol checks don't fail when given a null string
Hi! On Sun, 2009-03-22 at 13:23:45 +0100, Raphael Hertzog wrote: > On Fri, 20 Mar 2009, Bill Allombert wrote: > > > This is easy to fix by checking (s==end), or by setting errno=0 before > > > calling strto* functions, and checking (errno!=0) afterwards (see the > > > recently-updated manpage example). Given enough time, I will > > > eventually provide a patch for this, but I don't know that I'll get to > > > it anytime soon.. > > > > The attached patch should fix that and is conformant with man strtoul: > > > >In particular, if *nptr is not '\0' but **endptr is '\0' on return, > > the > >entire string is valid. > > Your patch doesn't compile: > ../../src/main.c: In function ‘setinteger’: > ../../src/main.c:296: error: invalid type argument of ‘unary *’ (have ‘int’) > make[3]: *** [main.o] Erreur 1 > > You added an unwanted "*" in the check. I'll commit it soon after tests. Before closing this bug, I'd like to get all other instances of strto* in the source tree reviewed, for this kind of missing checks or other flacky usage. regards, guillem -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#386197: dpkg: strtol checks don't fail when given a null string
On Fri, 20 Mar 2009, Bill Allombert wrote: > > This is easy to fix by checking (s==end), or by setting errno=0 before > > calling strto* functions, and checking (errno!=0) afterwards (see the > > recently-updated manpage example). Given enough time, I will > > eventually provide a patch for this, but I don't know that I'll get to > > it anytime soon.. > > The attached patch should fix that and is conformant with man strtoul: > >In particular, if *nptr is not '\0' but **endptr is '\0' on return, the >entire string is valid. Your patch doesn't compile: ../../src/main.c: In function ‘setinteger’: ../../src/main.c:296: error: invalid type argument of ‘unary *’ (have ‘int’) make[3]: *** [main.o] Erreur 1 You added an unwanted "*" in the check. I'll commit it soon after tests. Cheers, -- Raphaël Hertzog Contribuez à Debian et gagnez un cahier de l'admin Debian Lenny : http://www.ouaza.com/wp/2009/03/02/contribuer-a-debian-gagner-un-livre/ -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#386197: dpkg: strtol checks don't fail when given a null string
tags 386197 patch
quit
On Tue, Sep 05, 2006 at 03:30:56PM -0400, Justin Pryzby wrote:
> Package: dpkg
> Version: 1.13.21
> Severity: normal
>
> $ sudo dpkg --abort-after '' -i /
> dpkg-split: error reading /: Is a directory
> dpkg: error processing / (--install):
> subprocess dpkg-split returned error exit status 2
> dpkg: too many errors, stopping
> Errors were encountered while processing:
> /
> Processing was halted because there were too many errors.
>
> This is easy to fix by checking (s==end), or by setting errno=0 before
> calling strto* functions, and checking (errno!=0) afterwards (see the
> recently-updated manpage example). Given enough time, I will
> eventually provide a patch for this, but I don't know that I'll get to
> it anytime soon..
The attached patch should fix that and is conformant with man strtoul:
In particular, if *nptr is not '\0' but **endptr is '\0' on return, the
entire string is valid.
Cheers,
--
Bill.
Imagine a large red swirl here.
diff --git a/src/main.c b/src/main.c
index c892af3..ca1ba25 100644
--- a/src/main.c
+++ b/src/main.c
@@ -293,7 +293,7 @@ static void setinteger(const struct cmdinfo *cip, const
char *value) {
char *ep;
v= strtoul(value,&ep,0);
- if (*ep || v > INT_MAX)
+ if (!*value || **ep || v > INT_MAX)
badusage(_("invalid integer for --%s: `%.250s'"),cip->olong,value);
*cip->iassignto= v;
}
Bug#386197: dpkg: strtol checks don't fail when given a null string
Package: dpkg Version: 1.13.21 Severity: normal $ sudo dpkg --abort-after '' -i / dpkg-split: error reading /: Is a directory dpkg: error processing / (--install): subprocess dpkg-split returned error exit status 2 dpkg: too many errors, stopping Errors were encountered while processing: / Processing was halted because there were too many errors. This is easy to fix by checking (s==end), or by setting errno=0 before calling strto* functions, and checking (errno!=0) afterwards (see the recently-updated manpage example). Given enough time, I will eventually provide a patch for this, but I don't know that I'll get to it anytime soon.. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
