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

time parsing issues in cli-cmd-parser.c config_parse #4355

Open
eggert opened this issue May 23, 2024 · 0 comments · May be fixed by #4356
Open

time parsing issues in cli-cmd-parser.c config_parse #4355

eggert opened this issue May 23, 2024 · 0 comments · May be fixed by #4356

Comments

@eggert
Copy link

eggert commented May 23, 2024

Description of problem:
In cli/src/cli-cmd-parserlc, config_parse uses strftime's %s format in an unportable way. It calls strftime %s with a struct tm that may have out-of-range values, which has unspecified behavior according to POSIX. There is also a time zone portability issue.

Instead of calling strftime with %s to format a struct tm and then convert the resulting time_t value to a string suitable for giving to sprintf %s, the code should simply call mktime and check for mktime failures, and then us sprintf %lld to format the time_t. I plan to issue a pull request shortly about this.

The exact command to reproduce the issue:
I found the problem by code inspection. You should be able to reproduce abug by using wayyyy out-of-range dates (e.g., year == 2147485547) and/or in non-UTC timezones. Not sure it's worth the trouble to come up with test cases.

The full output of the command that failed:
See above.

Expected results:
See above.

Mandatory info:
- The output of the gluster volume info command:
Bug found by code inspection.

- The output of the gluster volume status command:
Bug found by code inspection.

- The output of the gluster volume heal command:
Bug found by code inspection.

**- Provide logs present on following locations of client and server nodes -
Bug found by code inspection.

**- Is there any crash ? Provide the backtrace and coredump
There could be a crash in theory, with out-of-range years. The call to strftime does not necessarily conform to POSIX.1-2017 so the behavior is unspecified. However, in practice the behavior will most likely just go ahead with a wildly-wrong timestamp.

Additional info:

- The operating system / glusterfs version:
Any.

Note: Please hide any confidential data which you don't want to share in public like IP address, file name, hostname or any other configuration

eggert added a commit to eggert/glusterfs that referenced this issue May 23, 2024
cli/src/cli-cmd-parser.c's config_parse was calling strftime
with a struct tm that may have tm_year that is out of range
for strftime, yielding unspecified and/or undefined behavior.
Fix this by using mktime instead of strftime, as mktime's
behavior is well-defined for out-of-range values.  This also
fixes a portability issue with strftime %s and time zones.

Fixes: gluster#4355

Signed-off-by: Paul Eggert <[email protected]>
@eggert eggert linked a pull request May 23, 2024 that will close this issue
eggert added a commit to eggert/glusterfs that referenced this issue May 23, 2024
Adjust patch to pass clang-format.

Fixes: gluster#4355

Signed-off-by: Paul Eggert <[email protected]>
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 a pull request may close this issue.

1 participant