Hi Hector, On Thu, Oct 24, 2019 at 03:22:43PM +0100, Hector Marco wrote: > Hello Daniel, > > Something went wrong in my last email, what I wanted to say is: > > The patch prevents that "cur_len" underflows. No negative values for > "cur_len" so no way to underflow the "cur_len" variable and therefore no > vulnerability.
First of all cur_len is unsigned. So, it does not get negative values at all. Though even it was signed I cannot see where in the code it can get negative value. Am I missing something? Daniel > Hector. > > > On 24/10/2019 15:13, Hector Marco wrote: > > Hello Daniel, > > > > The patch prevents that "cur_len" underflows. No negative values for > > "cur_len" so way to underflow the "cur_len" variable and therefore > > > > I hope this helps, > > > > Hector. > > > > > > > > On 23/10/2019 11:14, Daniel Kiper wrote: > >> On Fri, Oct 18, 2019 at 02:39:01PM +0200, Javier Martinez Canillas wrote: > >>> From: Hector Marco-Gisbert <hecma...@upv.es> > >>> > >>> This patch fixes two integer underflows at: > >>> * grub-core/lib/crypto.c > >>> * grub-core/normal/auth.c > >>> > >>> Resolves: CVE-2015-8370 > >>> > >>> Signed-off-by: Hector Marco-Gisbert <hecma...@upv.es> > >>> Signed-off-by: Ismael Ripoll-Ripoll <irip...@disca.upv.es> > >>> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> > >>> --- > >>> > >>> grub-core/lib/crypto.c | 2 +- > >>> grub-core/normal/auth.c | 2 +- > >>> 2 files changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/grub-core/lib/crypto.c b/grub-core/lib/crypto.c > >>> index ca334d5a40e..e6c78d16d39 100644 > >>> --- a/grub-core/lib/crypto.c > >>> +++ b/grub-core/lib/crypto.c > >>> @@ -468,7 +468,7 @@ grub_password_get (char buf[], unsigned buf_size) > >>> break; > >>> } > >>> > >>> - if (key == '\b') > >>> + if (key == '\b' && cur_len) > >>> { > >>> if (cur_len) > >>> cur_len--; > >>> diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c > >>> index 6be678c0de1..c35ce972473 100644 > >>> --- a/grub-core/normal/auth.c > >>> +++ b/grub-core/normal/auth.c > >>> @@ -172,7 +172,7 @@ grub_username_get (char buf[], unsigned buf_size) > >>> break; > >>> } > >>> > >>> - if (key == GRUB_TERM_BACKSPACE) > >>> + if (key == GRUB_TERM_BACKSPACE && cur_len) > >>> { > >>> if (cur_len) > >>> { > >> > >> TBH, I do not understand how this patch helps. It only delays continue > >> execution to the next "if (!grub_isprint (key))" if cur_len == 0. > >> > >> Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel