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

sscanf: Fix integer overflow with sscanf field width #31

Closed
frno7 opened this issue Sep 7, 2019 · 3 comments
Closed

sscanf: Fix integer overflow with sscanf field width #31

frno7 opened this issue Sep 7, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@frno7
Copy link
Owner

frno7 commented Sep 7, 2019

Fix 5380975 where sscanf overflows integers with simple strings such as dates. As an example, consider

        int r = sscanf("20190523123456", "%4d%2d%2d%2d%2d%2d",
                &year, &month, &day,
                &hour, &minute, &second);

        printk("%d %04d-%02d-%2d %02d:%02d:%02d\n",
                r,
                year, month, day,
                hour, minute, second);

On a 32-bit machine this prints

        6 0000-05-23 12:34:56

where the year is zero, and not 2019 as expected. The reason is that sscanf attempts to read 20190523123456 as a whole integer, and then divide it with 1010 to obtain 2019, which obviously fails. Of course, 64-bit machines fail similarly on longer numerical strings.

The idea for a fix is to have a variant of _parse_integer() called _parse_integer_end(), with the ability to stop consuming digits. The functions

        simple_{strtol,strtoll,strtoul,strtoull}()

now have the corresponding

        sscanf_{strtol,strtoll,strtoul,strtoull}()

taking a field width into account. Perhaps such a fix could be a starting-point to clean-up the integer parsers? Also, it seems to be a good idea to make a test suite for sscanf.

@frno7 frno7 added the bug Something isn't working label Sep 7, 2019
@frno7
Copy link
Owner Author

frno7 commented Sep 7, 2019

Commit d3638dc is a provisional fix. Post to the kernel mailing list. Reply from Jan Beulich.

@frno7
Copy link
Owner Author

frno7 commented Sep 7, 2019

These are the affected lines when reading the ROMVER file:

linux/arch/mips/ps2/rom.c

Lines 381 to 384 in 044d39c

return r > 0 && sscanf(buffer, "%4x%c%c%4d%2d%2d",
&v.number, &v.region, &v.type,
&v.date.year, &v.date.month, &v.date.day) == 6 ?
v : (struct rom_ver) { .region = '-', .type = '-' };

@frno7 frno7 mentioned this issue Sep 7, 2019
@frno7
Copy link
Owner Author

frno7 commented Dec 30, 2021

This bug has been fixed in commit 900fdc4.

@frno7 frno7 closed this as completed Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant