* inet_pton problem @ 2013-10-19 20:57 Paul Schutte 2013-10-20 2:22 ` Rich Felker 0 siblings, 1 reply; 6+ messages in thread From: Paul Schutte @ 2013-10-19 20:57 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 381 bytes --] Hi, I came across this and believe it is a bug. I have found that when you set str to an IPv4 addr of the from "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead of returning a 0 to indicate an invalid IPv6 string, it is converted to gibberish. inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I understand the specification correctly. Regards Paul [-- Attachment #2: Type: text/html, Size: 560 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: inet_pton problem 2013-10-19 20:57 inet_pton problem Paul Schutte @ 2013-10-20 2:22 ` Rich Felker 2013-10-20 9:50 ` Paul Schutte 0 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2013-10-20 2:22 UTC (permalink / raw) To: musl On Sat, Oct 19, 2013 at 10:57:00PM +0200, Paul Schutte wrote: > Hi, > > I came across this and believe it is a bug. > > I have found that when you set str to an IPv4 addr of the from > "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead of > returning a 0 to indicate an invalid IPv6 string, it is converted to > gibberish. From what I can tell, it's not converted to gibberish; instead, it's wrongly returning an error (-1) instead of a result indicating an invalid input string (0). One could argue that it's a programming error not to check this, but inet_pton should not have any reason to return -1 if the first argument (af) is valid, so one could also argue that such checks would be extraneous bloat. > inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I understand the > specification correctly. Agreed. Rich ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: inet_pton problem 2013-10-20 2:22 ` Rich Felker @ 2013-10-20 9:50 ` Paul Schutte 2013-10-20 12:26 ` Paul Schutte 0 siblings, 1 reply; 6+ messages in thread From: Paul Schutte @ 2013-10-20 9:50 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 3018 bytes --] Hi Rich, I agree with you, especially about the bloat part. They (haproxy) actually use this function to determine whether the address they have is a valid IPv6 address. They pass in either a valid IPv4 or IPv6 address and then rely on this function to determine which they have (assuming a return value of 0). After reading the spec more carefully I realise that -1 should be returned only when the address family is not AF_INET or AF_INET6. By changing the return value in the IPv6 code to 0 instead of -1, we could get the correct behaviour without any extra code. Here is a patch to try and save you a bit of work: --- a/musl/src/network/inet_pton.c +++ b/musl/src/network/inet_pton.c @@ -46,24 +46,24 @@ if (!s[1]) break; continue; } - if (hexval(s[0])<0) return -1; + if (hexval(s[0])<0) return 0; while (s[0]=='0' && s[1]=='0') s++; for (v=j=0; j<5 && (d=hexval(s[j]))>=0; j++) v=16*v+d; - if (v > 65535) return -1; + if (v > 65535) return 0; ip[i] = v; if (!s[j]) { - if (brk<0 && i!=7) return -1; + if (brk<0 && i!=7) return 0; break; } if (i<7) { if (s[j]==':') continue; - if (s[j]!='.') return -1; + if (s[j]!='.') return 0; need_v4=1; i++; break; } - return -1; + return 0; } if (brk>=0) { memmove(ip+brk+7-i, ip+brk, 2*(i+1-brk)); @@ -73,6 +73,6 @@ *a++ = ip[j]>>8; *a++ = ip[j]; } - if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return -1; + if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; return 1; } Regards Paul On Sun, Oct 20, 2013 at 4:22 AM, Rich Felker <dalias@aerifal.cx> wrote: > On Sat, Oct 19, 2013 at 10:57:00PM +0200, Paul Schutte wrote: > > Hi, > > > > I came across this and believe it is a bug. > > > > I have found that when you set str to an IPv4 addr of the from > > "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead of > > returning a 0 to indicate an invalid IPv6 string, it is converted to > > gibberish. > > From what I can tell, it's not converted to gibberish; instead, it's > wrongly returning an error (-1) instead of a result indicating an > invalid input string (0). One could argue that it's a programming > error not to check this, but inet_pton should not have any reason to > return -1 if the first argument (af) is valid, so one could also argue > that such checks would be extraneous bloat. > > > inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I understand > the > > specification correctly. > > Agreed. > > Rich > [-- Attachment #2: Type: text/html, Size: 3972 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: inet_pton problem 2013-10-20 9:50 ` Paul Schutte @ 2013-10-20 12:26 ` Paul Schutte 2013-10-20 15:30 ` Paul Schutte 0 siblings, 1 reply; 6+ messages in thread From: Paul Schutte @ 2013-10-20 12:26 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 4158 bytes --] Hi Rich, Unfortunately this is not the complete fix. Haproxy still complains about invalid networks. The following seems to fix the problem without adding too much bloat: --- a/musl/src/network/inet_pton.c +++ b/musl/src/network/inet_pton.c @@ -14,11 +14,11 @@ return -1; } -int inet_pton(int af, const char *restrict s, void *restrict a0) +int inet_pton(int af, const char *restrict s0, void *restrict a0) { uint16_t ip[8]; unsigned char *a = a0; - const char *z; + const char *z,*s = s0; unsigned long x; int i, j, v, d, brk=-1, need_v4=0; @@ -73,6 +73,10 @@ *a++ = ip[j]>>8; *a++ = ip[j]; } + + /* There must have been valid IPv6 preceding IPv4 dotted-quad */ + if (s==s0) return 0; + if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; return 1; } Regards Paul On Sun, Oct 20, 2013 at 11:50 AM, Paul Schutte <sjpschutte@gmail.com> wrote: > Hi Rich, > > I agree with you, especially about the bloat part. > > They (haproxy) actually use this function to determine whether the address > they have is a valid IPv6 address. > They pass in either a valid IPv4 or IPv6 address and then rely on this > function to determine which they have (assuming a return value of 0). > > After reading the spec more carefully I realise that -1 should be returned > only when the address family is not AF_INET or AF_INET6. > > By changing the return value in the IPv6 code to 0 instead of -1, we could > get the correct behaviour without any extra code. > > Here is a patch to try and save you a bit of work: > > --- a/musl/src/network/inet_pton.c > +++ b/musl/src/network/inet_pton.c > @@ -46,24 +46,24 @@ > if (!s[1]) break; > continue; > } > - if (hexval(s[0])<0) return -1; > + if (hexval(s[0])<0) return 0; > while (s[0]=='0' && s[1]=='0') s++; > for (v=j=0; j<5 && (d=hexval(s[j]))>=0; j++) > v=16*v+d; > - if (v > 65535) return -1; > + if (v > 65535) return 0; > ip[i] = v; > if (!s[j]) { > - if (brk<0 && i!=7) return -1; > + if (brk<0 && i!=7) return 0; > break; > } > if (i<7) { > if (s[j]==':') continue; > - if (s[j]!='.') return -1; > + if (s[j]!='.') return 0; > need_v4=1; > i++; > break; > } > - return -1; > + return 0; > } > if (brk>=0) { > memmove(ip+brk+7-i, ip+brk, 2*(i+1-brk)); > @@ -73,6 +73,6 @@ > *a++ = ip[j]>>8; > *a++ = ip[j]; > } > - if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return -1; > + if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; > return 1; > } > > Regards > Paul > > > > On Sun, Oct 20, 2013 at 4:22 AM, Rich Felker <dalias@aerifal.cx> wrote: > >> On Sat, Oct 19, 2013 at 10:57:00PM +0200, Paul Schutte wrote: >> > Hi, >> > >> > I came across this and believe it is a bug. >> > >> > I have found that when you set str to an IPv4 addr of the from >> > "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead of >> > returning a 0 to indicate an invalid IPv6 string, it is converted to >> > gibberish. >> >> From what I can tell, it's not converted to gibberish; instead, it's >> wrongly returning an error (-1) instead of a result indicating an >> invalid input string (0). One could argue that it's a programming >> error not to check this, but inet_pton should not have any reason to >> return -1 if the first argument (af) is valid, so one could also argue >> that such checks would be extraneous bloat. >> >> > inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I understand >> the >> > specification correctly. >> >> Agreed. >> >> Rich >> > > [-- Attachment #2: Type: text/html, Size: 5511 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: inet_pton problem 2013-10-20 12:26 ` Paul Schutte @ 2013-10-20 15:30 ` Paul Schutte 2013-10-20 16:45 ` Paul Schutte 0 siblings, 1 reply; 6+ messages in thread From: Paul Schutte @ 2013-10-20 15:30 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 5280 bytes --] Hi, My previous attempt still left the door open for the ":192.168.1.1" case to sneak through. The following handled everything I could dream up correctly: --- a/musl/src/network/inet_pton.c +++ b/musl/src/network/inet_pton.c @@ -14,11 +14,11 @@ return -1; } -int inet_pton(int af, const char *restrict s, void *restrict a0) +int inet_pton(int af, const char *restrict s0, void *restrict a0) { uint16_t ip[8]; unsigned char *a = a0; - const char *z; + const char *z,*s = s0; unsigned long x; int i, j, v, d, brk=-1, need_v4=0; @@ -73,6 +73,10 @@ *a++ = ip[j]>>8; *a++ = ip[j]; } + + /* IPv4 dotted-quad should have valid IPv6 in front*/ + if ((s-s0) <2) return 0; + if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; return 1; } On Sun, Oct 20, 2013 at 2:26 PM, Paul Schutte <sjpschutte@gmail.com> wrote: > Hi Rich, > > Unfortunately this is not the complete fix. > > Haproxy still complains about invalid networks. > > The following seems to fix the problem without adding too much bloat: > > --- a/musl/src/network/inet_pton.c > +++ b/musl/src/network/inet_pton.c > @@ -14,11 +14,11 @@ > return -1; > } > > -int inet_pton(int af, const char *restrict s, void *restrict a0) > +int inet_pton(int af, const char *restrict s0, void *restrict a0) > { > uint16_t ip[8]; > unsigned char *a = a0; > - const char *z; > + const char *z,*s = s0; > unsigned long x; > int i, j, v, d, brk=-1, need_v4=0; > > @@ -73,6 +73,10 @@ > *a++ = ip[j]>>8; > *a++ = ip[j]; > } > + > + /* There must have been valid IPv6 preceding IPv4 dotted-quad */ > + if (s==s0) return 0; > + > if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; > return 1; > } > > > > Regards > Paul > > > > On Sun, Oct 20, 2013 at 11:50 AM, Paul Schutte <sjpschutte@gmail.com>wrote: > >> Hi Rich, >> >> I agree with you, especially about the bloat part. >> >> They (haproxy) actually use this function to determine whether the >> address they have is a valid IPv6 address. >> They pass in either a valid IPv4 or IPv6 address and then rely on this >> function to determine which they have (assuming a return value of 0). >> >> After reading the spec more carefully I realise that -1 should be >> returned only when the address family is not AF_INET or AF_INET6. >> >> By changing the return value in the IPv6 code to 0 instead of -1, we >> could get the correct behaviour without any extra code. >> >> Here is a patch to try and save you a bit of work: >> >> --- a/musl/src/network/inet_pton.c >> +++ b/musl/src/network/inet_pton.c >> @@ -46,24 +46,24 @@ >> if (!s[1]) break; >> continue; >> } >> - if (hexval(s[0])<0) return -1; >> + if (hexval(s[0])<0) return 0; >> while (s[0]=='0' && s[1]=='0') s++; >> for (v=j=0; j<5 && (d=hexval(s[j]))>=0; j++) >> v=16*v+d; >> - if (v > 65535) return -1; >> + if (v > 65535) return 0; >> ip[i] = v; >> if (!s[j]) { >> - if (brk<0 && i!=7) return -1; >> + if (brk<0 && i!=7) return 0; >> break; >> } >> if (i<7) { >> if (s[j]==':') continue; >> - if (s[j]!='.') return -1; >> + if (s[j]!='.') return 0; >> need_v4=1; >> i++; >> break; >> } >> - return -1; >> + return 0; >> } >> if (brk>=0) { >> memmove(ip+brk+7-i, ip+brk, 2*(i+1-brk)); >> @@ -73,6 +73,6 @@ >> *a++ = ip[j]>>8; >> *a++ = ip[j]; >> } >> - if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return -1; >> + if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; >> return 1; >> } >> >> Regards >> Paul >> >> >> >> On Sun, Oct 20, 2013 at 4:22 AM, Rich Felker <dalias@aerifal.cx> wrote: >> >>> On Sat, Oct 19, 2013 at 10:57:00PM +0200, Paul Schutte wrote: >>> > Hi, >>> > >>> > I came across this and believe it is a bug. >>> > >>> > I have found that when you set str to an IPv4 addr of the from >>> > "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead of >>> > returning a 0 to indicate an invalid IPv6 string, it is converted to >>> > gibberish. >>> >>> From what I can tell, it's not converted to gibberish; instead, it's >>> wrongly returning an error (-1) instead of a result indicating an >>> invalid input string (0). One could argue that it's a programming >>> error not to check this, but inet_pton should not have any reason to >>> return -1 if the first argument (af) is valid, so one could also argue >>> that such checks would be extraneous bloat. >>> >>> > inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I >>> understand the >>> > specification correctly. >>> >>> Agreed. >>> >>> Rich >>> >> >> > [-- Attachment #2: Type: text/html, Size: 6960 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: inet_pton problem 2013-10-20 15:30 ` Paul Schutte @ 2013-10-20 16:45 ` Paul Schutte 0 siblings, 0 replies; 6+ messages in thread From: Paul Schutte @ 2013-10-20 16:45 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 5701 bytes --] The following handled everything I could dream up correctly: Evidently I need more sleep ... That breaks the common case of :: On Sun, Oct 20, 2013 at 5:30 PM, Paul Schutte <sjpschutte@gmail.com> wrote: > Hi, > > My previous attempt still left the door open for the ":192.168.1.1" case > to sneak through. > > The following handled everything I could dream up correctly: > > --- a/musl/src/network/inet_pton.c > +++ b/musl/src/network/inet_pton.c > @@ -14,11 +14,11 @@ > return -1; > } > > -int inet_pton(int af, const char *restrict s, void *restrict a0) > +int inet_pton(int af, const char *restrict s0, void *restrict a0) > { > uint16_t ip[8]; > unsigned char *a = a0; > - const char *z; > + const char *z,*s = s0; > unsigned long x; > int i, j, v, d, brk=-1, need_v4=0; > > @@ -73,6 +73,10 @@ > *a++ = ip[j]>>8; > *a++ = ip[j]; > } > + > + /* IPv4 dotted-quad should have valid IPv6 in front*/ > + if ((s-s0) <2) return 0; > + > if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; > return 1; > } > > > > On Sun, Oct 20, 2013 at 2:26 PM, Paul Schutte <sjpschutte@gmail.com>wrote: > >> Hi Rich, >> >> Unfortunately this is not the complete fix. >> >> Haproxy still complains about invalid networks. >> >> The following seems to fix the problem without adding too much bloat: >> >> --- a/musl/src/network/inet_pton.c >> +++ b/musl/src/network/inet_pton.c >> @@ -14,11 +14,11 @@ >> return -1; >> } >> >> -int inet_pton(int af, const char *restrict s, void *restrict a0) >> +int inet_pton(int af, const char *restrict s0, void *restrict a0) >> { >> uint16_t ip[8]; >> unsigned char *a = a0; >> - const char *z; >> + const char *z,*s = s0; >> unsigned long x; >> int i, j, v, d, brk=-1, need_v4=0; >> >> @@ -73,6 +73,10 @@ >> *a++ = ip[j]>>8; >> *a++ = ip[j]; >> } >> + >> + /* There must have been valid IPv6 preceding IPv4 dotted-quad */ >> + if (s==s0) return 0; >> + >> if (need_v4 && inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; >> return 1; >> } >> >> >> >> Regards >> Paul >> >> >> >> On Sun, Oct 20, 2013 at 11:50 AM, Paul Schutte <sjpschutte@gmail.com>wrote: >> >>> Hi Rich, >>> >>> I agree with you, especially about the bloat part. >>> >>> They (haproxy) actually use this function to determine whether the >>> address they have is a valid IPv6 address. >>> They pass in either a valid IPv4 or IPv6 address and then rely on this >>> function to determine which they have (assuming a return value of 0). >>> >>> After reading the spec more carefully I realise that -1 should be >>> returned only when the address family is not AF_INET or AF_INET6. >>> >>> By changing the return value in the IPv6 code to 0 instead of -1, we >>> could get the correct behaviour without any extra code. >>> >>> Here is a patch to try and save you a bit of work: >>> >>> --- a/musl/src/network/inet_pton.c >>> +++ b/musl/src/network/inet_pton.c >>> @@ -46,24 +46,24 @@ >>> if (!s[1]) break; >>> continue; >>> } >>> - if (hexval(s[0])<0) return -1; >>> + if (hexval(s[0])<0) return 0; >>> while (s[0]=='0' && s[1]=='0') s++; >>> for (v=j=0; j<5 && (d=hexval(s[j]))>=0; j++) >>> v=16*v+d; >>> - if (v > 65535) return -1; >>> + if (v > 65535) return 0; >>> ip[i] = v; >>> if (!s[j]) { >>> - if (brk<0 && i!=7) return -1; >>> + if (brk<0 && i!=7) return 0; >>> break; >>> } >>> if (i<7) { >>> if (s[j]==':') continue; >>> - if (s[j]!='.') return -1; >>> + if (s[j]!='.') return 0; >>> need_v4=1; >>> i++; >>> break; >>> } >>> - return -1; >>> + return 0; >>> } >>> if (brk>=0) { >>> memmove(ip+brk+7-i, ip+brk, 2*(i+1-brk)); >>> @@ -73,6 +73,6 @@ >>> *a++ = ip[j]>>8; >>> *a++ = ip[j]; >>> } >>> - if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return -1; >>> + if (need_v4 &&inet_pton(AF_INET, (void *)s, a-4) <= 0) return 0; >>> return 1; >>> } >>> >>> Regards >>> Paul >>> >>> >>> >>> On Sun, Oct 20, 2013 at 4:22 AM, Rich Felker <dalias@aerifal.cx> wrote: >>> >>>> On Sat, Oct 19, 2013 at 10:57:00PM +0200, Paul Schutte wrote: >>>> > Hi, >>>> > >>>> > I came across this and believe it is a bug. >>>> > >>>> > I have found that when you set str to an IPv4 addr of the from >>>> > "xxx.xxx.xxx.xxx' while the address family is AF_INET6, then instead >>>> of >>>> > returning a 0 to indicate an invalid IPv6 string, it is converted to >>>> > gibberish. >>>> >>>> From what I can tell, it's not converted to gibberish; instead, it's >>>> wrongly returning an error (-1) instead of a result indicating an >>>> invalid input string (0). One could argue that it's a programming >>>> error not to check this, but inet_pton should not have any reason to >>>> return -1 if the first argument (af) is valid, so one could also argue >>>> that such checks would be extraneous bloat. >>>> >>>> > inet_pton(AF_INET6, "192.168.1.1', &sa) should return 0 if I >>>> understand the >>>> > specification correctly. >>>> >>>> Agreed. >>>> >>>> Rich >>>> >>> >>> >> > [-- Attachment #2: Type: text/html, Size: 7551 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-20 16:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-10-19 20:57 inet_pton problem Paul Schutte 2013-10-20 2:22 ` Rich Felker 2013-10-20 9:50 ` Paul Schutte 2013-10-20 12:26 ` Paul Schutte 2013-10-20 15:30 ` Paul Schutte 2013-10-20 16:45 ` Paul Schutte
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).