Page MenuHomeFreeBSD

newsyslog: Wrap months when parsing 'W' rules, diff #2
ClosedPublic

Authored by gad on Dec 29 2021, 2:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 8 2024, 4:40 AM
Unknown Object (File)
Mar 8 2024, 4:40 AM
Unknown Object (File)
Mar 8 2024, 4:24 AM
Unknown Object (File)
Jan 13 2024, 3:09 AM
Unknown Object (File)
Dec 22 2023, 11:32 PM
Unknown Object (File)
Dec 17 2023, 1:29 PM
Unknown Object (File)
Dec 12 2023, 4:36 AM
Unknown Object (File)
Sep 18 2023, 8:17 AM
Subscribers

Details

Summary

When parsing a rule to rotate log files on a specific week day,
parseDWM() can advance the time to the next week. If the next week is
in the next month, then tm_mon is incremented. However, the increment
was failing to handle the wraparound from December to January, so when
parsing a rule during the last week of the December, the month would
advance to month 12. This triggered an out-of-bounds read of the
mtab[] array in days_pmonth() after parseDWM() returned.

The default rule for /var/log/weekly.log triggers this during the
last week of December each year.

Reported by: CHERI
Obtained from: CheriBSD
Sponsored by: The University of Cambridge, Google Inc.

This diff will also increment the year-value when it needs to
reset the month-value from 12 to 0.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

gad created this object with edit policy "committers (Project)".
gad requested review of this revision.Dec 29 2021, 2:21 AM

I've already done detailed testing of this, both before and after applying the update. The problem reported did definitely exist before this change, and it's definitely fixed after applying this update. I only tested it on amd64, but it'd be pretty bizarre if this worked on amd64 and didn't work on some other HW.

This revision is now accepted and ready to land.Dec 29 2021, 2:30 AM

Yeah, this is correct. I think you can just rework a bit of the original log message to describe the proper fix, something like:

When parsing a rule to rotate log files on a specific week day,
parseDWM() can advance the time to the next week. If the next week is
in the next month, then tm_mon is incremented. However, the increment
was failing to handle the wraparound from December to January, so when
parsing a rule during the last week of the December, the month would
advance to month 12. This triggered an out-of-bounds read of the
mtab[] array in days_pmonth() after parseDWM() returned.  To fix, reset
the month to January and increment the year when the month increment
wraps.

The default rule for /var/log/weekly.log triggers this during the
last week of December each year.

For future reference, if you wanted to take over this patch and commit it yourself, a new review makes sense (there is also an action in Phab to "commandeer" a review to take over an existing review and then you can upload patches to it, etc.). However, if you want to just suggest an edit to the existing commit, we usually do that by just doing comments on the original review (you can do suggestions in a comment which gives you a chance to edit the new code inline).