This attempts to fix the places where we do the following where
arithmetic_expr may include unvalidated data:
X = grub_malloc(arithmetic_expr);
It accomplishes this by doing the arithmetic ahead of time using grub_add(),
grub_sub(), grub_mul() and testing for overflow before proceeding.
Among other issues, this fixes:
- allocation of integer overflow in grub_video_bitmap_create()
reported by Chris Coulson,
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in grub_squash_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in grub_ext2_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in read_section_as_string()
reported by Chris Coulson.
Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This modifies most of the places we do some form of:
X = malloc(Y * Z);
to use calloc(Y, Z) instead.
Among other issues, this fixes:
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in luks_recover_key()
reported by Chris Coulson,
- allocation of integer overflow in grub_lvm_detect()
reported by Chris Coulson.
Fixes: CVE-2020-14308
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
On some devices the ESC key is the hotkey to enter the BIOS/EFI setup
screen, making it really hard to time pressing it right. Besides that
ESC is also pretty hard to discover for a user who does not know it
will unhide the menu.
This commit makes F4, which was chosen because is not used as a hotkey
to enter the BIOS setup by any vendor, also interrupt sleeps / stop the
menu countdown.
This solves the ESC gets into the BIOS setup and also somewhat solves
the discoverability issue, but leaves the timing issue unresolved.
This commit fixes the timing issue by also adding support for keeping
SHIFT pressed during boot to stop the menu countdown. This matches
what Ubuntu is doing, which should also help with discoverability.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Coverity Scan reports that the grub_strrchr() function can return NULL if
the character is not found. Check if that's the case for dirfile pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.
The typical declarations of these functions follow this pattern:
long
strtol(const char * restrict nptr, char ** restrict endptr, int base);
Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change. (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)
The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.
This patch changes these declarations to follow this pattern instead:
long
strtol(const char * restrict nptr,
const char ** const restrict endptr,
int base);
This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error. (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)
It also means the typical use case changes to:
char *s = ...;
const char *end;
long l;
l = strtol(s, &end, 10);
Or even:
const char *p = str;
while (p && *p) {
long l = strtol(p, &p, 10);
...
}
This fixes 26 places where we discard our attempts at treating the data
safely by doing:
const char *p = str;
long l;
l = strtol(p, (char **)&ptr, 10);
It also adds 5 places where we do:
char *p = str;
while (p && *p) {
long l = strtol(p, (const char ** const)&p, 10);
...
/* more calls that need p not to be pointer-to-const */
}
While moderately distasteful, this is a better problem to have.
With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'. The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.
Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions. This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.
The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later). strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.
The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type. Quoting from C99, the
syntax is:
declarator:
pointer[opt] direct-declarator
direct-declarator:
identifier
( declarator )
direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
...
direct-declarator [ type-qualifier-list[opt] * ]
...
pointer:
* type-qualifier-list[opt]
* type-qualifier-list[opt] pointer
type-qualifier-list:
type-qualifier
type-qualifier-list type-qualifier
...
type-qualifier:
const
restrict
volatile
So the examples go like:
const char foo; // immutable object
const char *foo; // mutable pointer to object
char * const foo; // immutable pointer to mutable object
const char * const foo; // immutable pointer to immutable object
const char const * const foo; // XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
// pointer to immutable object
const char ** const foo; // immutable pointer to mutable pointer
// to immutable object
Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way. I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.
Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch implements a search for a specific configuration when the config
file is on a remoteserver. It uses the following order:
1) DHCP client UUID option.
2) MAC address (in lower case hexadecimal with dash separators);
3) IP (in upper case hexadecimal) or IPv6;
4) The original grub.cfg file.
This procedure is similar to what is used by pxelinux and yaboot:
http://www.syslinux.org/wiki/index.php/PXELINUX#config
It is enabled by default but can be disabled by setting the environment
variable "feature_net_search_cfg" to "n" in an embedded configuration.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The common datetime helper functions are currently included in the normal
module, but this makes any other module that calls these functions to have
a dependency with the normal module only for this reason.
Since the normal module does a lot of stuff, it calls functions from other
modules. But since other modules may depend on it for calling the datetime
helpers, this could lead to circular dependencies between modules.
As an example, when platform == xen the grub_get_datetime() function from
the datetime module calls to the grub_unixtime2datetime() helper function
from the normal module. Which leads to the following module dependency:
datetime -> normal
and send_dhcp_packet() from the net module calls the grub_get_datetime()
function, which leads to the following module dependency:
net -> datetime -> normal
but that means that the normal module is not allowed to depend on net or
any other module that depends on it due the transitive dependency caused
by datetime. A recent patch attempted to add support to fetch the config
file over the network, which leads to the following circular dependency:
normal -> net -> datetime -> normal
So having the datetime helpers in the normal module makes it quite fragile
and easy to add circular dependencies like these, that break the build due
the genmoddep.awk script catching the issues.
Fix this by taking the datetime helper functions out of the normal module
and instead add them to the datetime module itself. Besides fixing these
issues, it makes more sense to have these helper functions there anyways.
Reported-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Some terminals, like `grub-core/term/at_keyboard.c`, return `-1` in case
they are not ready yet.
if (! KEYBOARD_ISREADY (grub_inb (KEYBOARD_REG_STATUS)))
return -1;
Currently, that is treated as a key press, and the menu time-out is
cancelled/cleared. This is unwanted, as the boot is stopped and the user
manually has to select a menu entry. Therefore, adapt the condition to
require the key value also to be greater than 0.
`GRUB_TERM_NO_KEY` is defined as 0, so the condition could be collapsed
to greater or equal than (≥) 0, but the compiler will probably do that
for us anyway, so keep the cases separate for clarity.
This is tested with coreboot, the GRUB default payload, and the
configuration file `grub.cfg` below.
For GRUB:
$ ./autogen.sh
$ ./configure --with-platform=coreboot
$ make -j`nproc`
$ make default_payload.elf
For coreboot:
$ more grub.cfg
serial --unit 0 --speed 115200
set timeout=5
menuentry 'halt' {
halt
}
$ build/cbfstool build/coreboot.rom add-payload \
-f /dev/shm/grub/default_payload.elf -n fallback/payload -c lzma
$ build/cbfstool build/coreboot.rom add -f grub.cfg -n etc/grub.cfg -t raw
$ qemu-system-x86_64 --version
QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-2+b1)
Copyright (c) 2003-2018 Fabrice Bellard and the QEMU Project developers
$ qemu-system-x86_64 -M pc -bios build/coreboot.rom -serial stdio -nic none
Currently, the time-out is cancelled/cleared. With the commit, it is not.
With a small GRUB payload, this the problem is also reproducible on the
ASRock E350M1.
Link: http://lists.gnu.org/archive/html/grub-devel/2019-01/msg00037.html
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Let's provide file type info to the I/O layer. This way verifiers
framework and its users will be able to differentiate files and verify
only required ones.
This is preparatory patch.
Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
When running grub in a VGA console of a KVM pseries guest on PowerPC,
you can see the cursor sweeping over the whole line when entering a
character in editor mode. This is visible because grub always refreshes
the whole line when entering a character in editor mode, and drawing
characters is quite a slow operation with the firmware used for the
powerpc pseries guests (SLOF).
To avoid this ugliness, the cursor should be disabled when refreshing
the screen contents during update_screen().
Signed-off-by: Thomas Huth <thuth@redhat.com>
If configfile is relative pathname, extend it with current ($root) so its
interpretation does not change if $root is changed later.
Suggested by Vladimir Serbienko.
Defalut font color on PC console seems to be light-gray; this is
what user also gets in rescue prompt and what is defined as
GRUB_TERM_DEFAULT_NORMAL_COLOR. But normal.mod defaults to white.
This makes unpleasant visual effect as colors are changed after kernel
is booted.
Use the same color eveywhere for consistency and default to light-gray
as this is also what at least Linux kernel is using by default.
Add a new timeout_style environment variable and a corresponding
GRUB_TIMEOUT_STYLE configuration key for grub-mkconfig. This
controls hidden-timeout handling more simply than the previous
arrangements, and pressing any hotkeys associated with menu entries
during the hidden timeout will now boot the corresponding menu entry
immediately.
GRUB_HIDDEN_TIMEOUT=<non-empty> + GRUB_TIMEOUT=<non-zero> now
generates a warning, and if it shows the menu it will do so as if
the second timeout were not present. Other combinations are
translated into reasonable equivalents.
Peter Jones