Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cve/meltdown: use the snprintf function to prevent buffer overflow #1079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KunWuChan
Copy link

@KunWuChan KunWuChan commented Sep 20, 2023

Use the snprintf function instead of sprintf in the meltdown.c file to prevent buffer overflow.
Cause the length of the release in a struct utsname is unspecified.

struct utsname {
char sysname[]; /* Operating system name (e.g., "Linux") /
char nodename[]; /
Name within communications network
to which the node is attached, if any /
char release[]; /
Operating system release
(e.g., "2.6.28") /
char version[]; /
Operating system version /
char machine[]; /
Hardware type identifier /
#ifdef _GNU_SOURCE
char domainname[]; /
NIS or YP domain name */
#endif
};

Signed-off-by: Kunwu Chan [email protected]

Use the snprintf function instead of sprintf in the meltdown.c file to prevent buffer overflow.
Cause the  length of the release in a struct utsname is unspecified.
struct utsname {
               char sysname[];    /* Operating system name (e.g., "Linux") */
               char nodename[];   /* Name within communications network
                                     to which the node is attached, if any */
               char release[];    /* Operating system release
                                     (e.g., "2.6.28") */
               char version[];    /* Operating system version */
               char machine[];    /* Hardware type identifier */
           #ifdef _GNU_SOURCE
               char domainname[]; /* NIS or YP domain name */
           #endif
           };
@richiejp
Copy link
Contributor

It appears that the struct member in question has its length specified by the libc implementation and can be accessed with sizeof. This is stated in the man page.

@@ -281,7 +281,7 @@ find_kernel_symbol(const char *name)

if (uname(&utsname) < 0)
tst_brk(TBROK | TERRNO, "uname");
sprintf(systemmap, "/boot/System.map-%s", utsname.release);
snprintf(systemmap, sizeof(systemmap), "/boot/System.map-%s", utsname.release);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

systemmap is supposed to be null terminated. So you must reserve one byte for null and set it to zero.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually as far as I can tell snprintf() always null terminates the output buffer, unless of course the buffer size passed is 0. So this patch looks good to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I've update the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants