From 9789069c0de77ecdafb1ff3b909c0f65ed9267c1 Mon Sep 17 00:00:00 2001 From: Celine Lee Date: Fri, 25 Mar 2022 22:29:18 -0400 Subject: [PATCH] Modify casting in protection against underflow (#1514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dear Ventoy community – Our team is working with your code and we noticed this if logical expression: if (len - 1 - (int)(long)(pos - pwdstr) != 32) We studied the surrounding code. We believe we understand the intention of the type casts in the above if statement. It seems they were meant to ensure an underflow doesn’t occur by the subtraction between to char pointers, which is a great catch (!). However, we believe the way the type casts are structured, the code is not actually protecting against such underflow because pwdstr isn’t cast into a signed long until after the subtraction occurs. To properly protect this code against underflow, we believe it should be changed to something like the following: if (len - 1 - ((long)pos – (long)pwdstr) != 32) Or, to enhance readability for junior engineers who may not know that the “long” type cast is implicitly of a signed integer type, we could include the `signed` keyword for added verbosity: if (len - 1 - ((signed long)pos – (signed long)pwdstr) != 32) Thank you! --- GRUB2/MOD_SRC/grub-2.04/grub-core/ventoy/ventoy_plugin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GRUB2/MOD_SRC/grub-2.04/grub-core/ventoy/ventoy_plugin.c b/GRUB2/MOD_SRC/grub-2.04/grub-core/ventoy/ventoy_plugin.c index 841327ae..b7152f9a 100644 --- a/GRUB2/MOD_SRC/grub-2.04/grub-core/ventoy/ventoy_plugin.c +++ b/GRUB2/MOD_SRC/grub-2.04/grub-core/ventoy/ventoy_plugin.c @@ -960,7 +960,7 @@ static int ventoy_plugin_parse_pwdstr(char *pwdstr, vtoy_password *pwd) return 1; } - if (len - 1 - (int)(long)(pos - pwdstr) != 32) + if (len - 1 - ((long)pos - (long)pwdstr) != 32) { if (NULL == pwd) grub_printf("Invalid md5 salt password format %s\n", pwdstr); return 1;