* [PATCH 1/2] misc: fix invalid character recongition in strto*l
@ 2016-04-27 20:42 Aaron Miller
0 siblings, 0 replies; 6+ messages in thread
From: Aaron Miller @ 2016-04-27 20:42 UTC (permalink / raw)
To: grub-devel
Would previously allow digits larger than the base and didn't check that
subtracting the difference from 0-9 to lowercase letters for characters
larger than 9 didn't result in a value lower than 9, which allowed the
parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
---
grub-core/kern/misc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 906d2c2..85ff109 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -391,10 +391,12 @@ grub_strtoull (const char *str, char **end, int base)
unsigned long digit;
digit = grub_tolower (*str) - '0';
+ if (digit < (unsigned long) base)
+ break;
if (digit > 9)
{
digit += '0' - 'a' + 10;
- if (digit >= (unsigned long) base)
+ if (digit >= (unsigned long) base || digit <= 9)
break;
}
--
2.8.0.rc2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] misc: fix invalid character recongition in strto*l
@ 2016-04-27 21:54 Aaron Miller
2016-04-28 0:53 ` Aaron Miller
0 siblings, 1 reply; 6+ messages in thread
From: Aaron Miller @ 2016-04-27 21:54 UTC (permalink / raw)
To: grub-devel
Would previously allow digits larger than the base and didn't check that
subtracting the difference from 0-9 to lowercase letters for characters
larger than 9 didn't result in a value lower than 9, which allowed the
parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
---
Missed the >= vs < in the previously sent patch (I caught this, but then
still mailed the broken patch)
grub-core/kern/misc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 906d2c2..1c0c913 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -391,10 +391,12 @@ grub_strtoull (const char *str, char **end, int
base)
unsigned long digit;
digit = grub_tolower (*str) - '0';
+ if (digit >= (unsigned long) base)
+ break;
if (digit > 9)
{
digit += '0' - 'a' + 10;
- if (digit >= (unsigned long) base)
+ if (digit >= (unsigned long) base || digit <= 9)
break;
}
--
2.8.0.rc2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: fix invalid character recongition in strto*l
2016-04-27 21:54 [PATCH 1/2] misc: fix invalid character recongition in strto*l Aaron Miller
@ 2016-04-28 0:53 ` Aaron Miller
2016-04-28 18:01 ` Andrei Borzenkov
0 siblings, 1 reply; 6+ messages in thread
From: Aaron Miller @ 2016-04-28 0:53 UTC (permalink / raw)
To: grub-devel
Would previously allow digits larger than the base and didn't check that
subtracting the difference from 0-9 to lowercase letters for characters
larger than 9 didn't result in a value lower than 9, which allowed the
parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
---
Need to move the out-of-base check to *after* the outside [0-9] handling
or this breaks.
grub-core/kern/misc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 906d2c2..3653d4d 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -394,9 +394,11 @@ grub_strtoull (const char *str, char **end, int
base)
if (digit > 9)
{
digit += '0' - 'a' + 10;
- if (digit >= (unsigned long) base)
+ if (digit >= (unsigned long) base || digit <= 9)
break;
}
+ if (digit >= (unsigned long) base)
+ break;
found = 1;
--
2.8.0.rc2
On 27 Apr 2016, at 14:54, Aaron Miller wrote:
> Would previously allow digits larger than the base and didn't check
> that
> subtracting the difference from 0-9 to lowercase letters for
> characters
> larger than 9 didn't result in a value lower than 9, which allowed the
> parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
> ---
>
> Missed the >= vs < in the previously sent patch (I caught this, but
> then still mailed the broken patch)
>
> grub-core/kern/misc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 906d2c2..1c0c913 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -391,10 +391,12 @@ grub_strtoull (const char *str, char **end, int
> base)
> unsigned long digit;
>
> digit = grub_tolower (*str) - '0';
> + if (digit >= (unsigned long) base)
> + break;
> if (digit > 9)
> {
> digit += '0' - 'a' + 10;
> - if (digit >= (unsigned long) base)
> + if (digit >= (unsigned long) base || digit <= 9)
> break;
> }
>
> --
> 2.8.0.rc2
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: fix invalid character recongition in strto*l
2016-04-28 0:53 ` Aaron Miller
@ 2016-04-28 18:01 ` Andrei Borzenkov
2016-04-29 19:12 ` Aaron Miller
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Borzenkov @ 2016-04-28 18:01 UTC (permalink / raw)
To: The development of GNU GRUB
28.04.2016 03:53, Aaron Miller пишет:
> Would previously allow digits larger than the base and didn't check that
> subtracting the difference from 0-9 to lowercase letters for characters
> larger than 9 didn't result in a value lower than 9, which allowed the
> parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
Does it cause any real problem (i.e. is it 2.02 material)?
> ---
>
> Need to move the out-of-base check to *after* the outside [0-9] handling
> or this breaks.
>
> grub-core/kern/misc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 906d2c2..3653d4d 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -394,9 +394,11 @@ grub_strtoull (const char *str, char **end, int base)
> if (digit > 9)
> {
> digit += '0' - 'a' + 10;
> - if (digit >= (unsigned long) base)
> + if (digit >= (unsigned long) base || digit <= 9)
base comparison becomes redundant here. And this needs comment
explaining digit <= 9 comparison for future reference.
> break;
> }
> + if (digit >= (unsigned long) base)
> + break;
>
> found = 1;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: fix invalid character recongition in strto*l
2016-04-28 18:01 ` Andrei Borzenkov
@ 2016-04-29 19:12 ` Aaron Miller
2016-04-29 19:19 ` Aaron Miller
0 siblings, 1 reply; 6+ messages in thread
From: Aaron Miller @ 2016-04-29 19:12 UTC (permalink / raw)
To: The development of GNU GRUB
On 28 Apr 2016, at 11:01, Andrei Borzenkov wrote:
> 28.04.2016 03:53, Aaron Miller пишет:
>> Would previously allow digits larger than the base and didn't check
>> that
>> subtracting the difference from 0-9 to lowercase letters for
>> characters
>> larger than 9 didn't result in a value lower than 9, which allowed
>> the
>> parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
>
> Does it cause any real problem (i.e. is it 2.02 material)?
>
With the later patch I sent (allowing port numbers including
"[ipv6:addr]:port"), it is needed to keep something like "0000]" from
being parsed as 6. I don't know of any case without that where it can
cause a problem -- would be any case where a number being read is
followed by any of `_^]\[
>> ---
>>
>> Need to move the out-of-base check to *after* the outside [0-9]
>> handling
>> or this breaks.
>>
>> grub-core/kern/misc.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>> index 906d2c2..3653d4d 100644
>> --- a/grub-core/kern/misc.c
>> +++ b/grub-core/kern/misc.c
>> @@ -394,9 +394,11 @@ grub_strtoull (const char *str, char **end, int
>> base)
>> if (digit > 9)
>> {
>> digit += '0' - 'a' + 10;
>> - if (digit >= (unsigned long) base)
>> + if (digit >= (unsigned long) base || digit <= 9)
>
> base comparison becomes redundant here. And this needs comment
> explaining digit <= 9 comparison for future reference.
>
Will resend with a comment -- this and the below are both needed when
for example base=8 and digit is '9' or '8' and should still be rejected.
digit <= 9 is needed to prevent the '` = 9, _ = 8, ^ = 7, ] = 6, \ = 5,
[ = 4' problem.
>> break;
>> }
>> + if (digit >= (unsigned long) base)
>> + break;
>>
>> found = 1;
>>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] misc: fix invalid character recongition in strto*l
2016-04-29 19:12 ` Aaron Miller
@ 2016-04-29 19:19 ` Aaron Miller
0 siblings, 0 replies; 6+ messages in thread
From: Aaron Miller @ 2016-04-29 19:19 UTC (permalink / raw)
To: The development of GNU GRUB
Would previously allow digits larger than the base and didn't check that
subtracting the difference from 0-9 to lowercase letters for characters
larger than 9 didn't result in a value lower than 9, which allowed the
parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
---
Added comment.
grub-core/kern/misc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 906d2c2..d181036 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -394,9 +394,13 @@ grub_strtoull (const char *str, char **end, int
base)
if (digit > 9)
{
digit += '0' - 'a' + 10;
- if (digit >= (unsigned long) base)
+ // digit <= 9 check is needed to keep chars larger than
+ // '9' but less than 'a' from being read as numbers
+ if (digit >= (unsigned long) base || digit <= 9)
break;
}
+ if (digit >= (unsigned long) base)
+ break;
found = 1;
--
2.8.0.rc2
On 29 Apr 2016, at 12:12, Aaron Miller wrote:
> On 28 Apr 2016, at 11:01, Andrei Borzenkov wrote:
>
>> 28.04.2016 03:53, Aaron Miller пишет:
>>> Would previously allow digits larger than the base and didn't check
>>> that
>>> subtracting the difference from 0-9 to lowercase letters for
>>> characters
>>> larger than 9 didn't result in a value lower than 9, which allowed
>>> the
>>> parses: ` = 9, _ = 8, ^ = 7, ] = 6, \ = 5, and [ = 4
>>
>> Does it cause any real problem (i.e. is it 2.02 material)?
>>
>
> With the later patch I sent (allowing port numbers including
> "[ipv6:addr]:port"), it is needed to keep something like "0000]" from
> being parsed as 6. I don't know of any case without that where it can
> cause a problem -- would be any case where a number being read is
> followed by any of `_^]\[
>
>>> ---
>>>
>>> Need to move the out-of-base check to *after* the outside [0-9]
>>> handling
>>> or this breaks.
>>>
>>> grub-core/kern/misc.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>>> index 906d2c2..3653d4d 100644
>>> --- a/grub-core/kern/misc.c
>>> +++ b/grub-core/kern/misc.c
>>> @@ -394,9 +394,11 @@ grub_strtoull (const char *str, char **end, int
>>> base)
>>> if (digit > 9)
>>> {
>>> digit += '0' - 'a' + 10;
>>> - if (digit >= (unsigned long) base)
>>> + if (digit >= (unsigned long) base || digit <= 9)
>>
>> base comparison becomes redundant here. And this needs comment
>> explaining digit <= 9 comparison for future reference.
>>
>
> Will resend with a comment -- this and the below are both needed when
> for example base=8 and digit is '9' or '8' and should still be
> rejected. digit <= 9 is needed to prevent the '` = 9, _ = 8, ^ = 7, ]
> = 6, \ = 5, [ = 4' problem.
>
>>> break;
>>> }
>>> + if (digit >= (unsigned long) base)
>>> + break;
>>>
>>> found = 1;
>>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-29 19:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-27 21:54 [PATCH 1/2] misc: fix invalid character recongition in strto*l Aaron Miller
2016-04-28 0:53 ` Aaron Miller
2016-04-28 18:01 ` Andrei Borzenkov
2016-04-29 19:12 ` Aaron Miller
2016-04-29 19:19 ` Aaron Miller
-- strict thread matches above, loose matches on Subject: below --
2016-04-27 20:42 Aaron Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.