While option --dev-mem can be convenient for testing purposes, it
could be abused by attackers to force dmidecode to read a malicious
file. Add a safety check on the type of the mem device file we are
asked to read from. If we are root and this isn't a character device
file, then something is fishy and we better stop.
For non-root users, reading from a regular file is OK and accepted.
Signed-off-by: Jean Delvare <jdelv...@suse.de>
---
Changes since v1:
* Moved the check to function mem_chunk() to close a race condition
noticed by my colleague Matthias Gerstner. A nice side effect is
that the check also covers the other utilities (biosdecode etc.)
now.
* Don't provide detailed information on failure, so as to not help
attackers.
util.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
--- dmidecode.orig/util.c
+++ dmidecode/util.c
@@ -173,18 +173,26 @@ static void safe_memcpy(void *dest, cons
*/
void *mem_chunk(off_t base, size_t len, const char *devmem)
{
- void *p;
+ struct stat statbuf;
+ void *p = NULL;
int fd;
#ifdef USE_MMAP
- struct stat statbuf;
off_t mmoffset;
void *mmp;
#endif
- if ((fd = open(devmem, O_RDONLY)) == -1)
+ /*
+ * Safety check: if running as root, devmem is expected to be a
+ * character device file.
+ */
+ if ((fd = open(devmem, O_RDONLY)) == -1
+ || fstat(fd, &statbuf) == -1
+ || (geteuid() == 0 && !S_ISCHR(statbuf.st_mode)))
{
- perror(devmem);
- return NULL;
+ fprintf(stderr, "Can't read memory from %s\n", devmem);
+ if (fd == -1)
+ return NULL;
+ goto out;
}
if ((p = malloc(len)) == NULL)
@@ -194,13 +202,6 @@ void *mem_chunk(off_t base, size_t len,
}
#ifdef USE_MMAP
- if (fstat(fd, &statbuf) == -1)
- {
- fprintf(stderr, "%s: ", devmem);
- perror("stat");
- goto err_free;
- }
-
/*
* mmap() will fail with SIGBUS if trying to map beyond the end of
* the file.
--
Jean Delvare
SUSE L3 Support
