From 9d90ef9e836a4e5206e4958396f0c03d2de33b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= Date: Thu, 13 Feb 2020 17:38:33 +0100 Subject: [PATCH] Fix timestamp handling in MDTM Resolves: rhbz#1567855 --- 0001-Fix-timestamp-handling-in-MDTM.patch | 151 ++++++++++++++++++++++ 0002-Drop-an-unused-global-variable.patch | 56 ++++++++ vsftpd.spec | 8 +- 3 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 0001-Fix-timestamp-handling-in-MDTM.patch create mode 100644 0002-Drop-an-unused-global-variable.patch diff --git a/0001-Fix-timestamp-handling-in-MDTM.patch b/0001-Fix-timestamp-handling-in-MDTM.patch new file mode 100644 index 0000000..3975bf3 --- /dev/null +++ b/0001-Fix-timestamp-handling-in-MDTM.patch @@ -0,0 +1,151 @@ +From 6a4dc470e569df38b8a7ea09ee6aace3c73b7353 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Wed, 28 Mar 2018 09:06:34 +0200 +Subject: [PATCH 1/2] Fix timestamp handling in MDTM + +There were two problems with the timestamp handling with MDTM: + +1. In vsf_sysutil_parse_time(), the `the_time.tm_isdst` attribute was + always set to 0, regardless of whether DST (daylight saving time) + is active on the given date or not. + + This made glibc shift the timestamp when DST was in fact active on + the given date, in an attempt to correct the discrepancy between + the given timestamp and the `tm_isdst` attribute. The shifting + produced incorrect results however. + + We fix this by setting `tm_isdst` to -1 to let glibc decide if DST + is active or not at the time of the timestamp. glibc won't touch + the timestamp then. + +2. vsftpd used to record the offset from UTC of the current timezone + in the global variable `s_timezone`. This variable was then + subtracted from the variable `the_time` in vsf_sysutil_setmodtime() + when the config option use_localtime=NO was set. This was done to + compensate for the fact that mktime(), used in + vsf_sysutil_parse_time(), expects a timestamp expressed as local + time, whereas vsftpd is dealing with universal time. + + However, this did not work in the case when the offset stored in + `s_timezone` did not match the timezone of the timestamp given to + mktime() - this happens when DST is active at the current time, but + DST is not active at the time of the timestamp, or vice versa. + + We fix this by subtracting the real timezone offset directly in + vsf_sysutil_parse_time(). + + Note that the `tm_gmtoff` attribute, used in this fix, is a + BSD/glic extension. However, using `tm_gmtoff` seems like the + simplest solution and we need to make this work only with glibc + anyway. + +The fix was tested in the following way. We checked that the timestamp +given to the MDTM command when setting modification time exactly +matches the timestamp received as response from MDTM when reading back +the modification time. Additionally, we checked that the modification +time was set correctly on the given file on disk. + +These two checks were performed under various conditions - all the +combinations of DST/non-DST system time, DST/non-DST modification +time, use_localtime=YES/NO. + +Note that (I think) this will still not work if the rules for when DST +is active change. For example, if DST is ever completely cancelled in +the Europe/Prague timezone, and vsftpd is dealing with a timestamp +from a time when DST was active, it will produce incorrect results. I +think we would need the full zone file to fix this, but the zone file +is hard to provide when we're chroot-ed. + +Resolves: rhbz#1567855 +--- + postlogin.c | 5 +++-- + sysutil.c | 17 ++++++++++------- + sysutil.h | 4 ++-- + 3 files changed, 15 insertions(+), 11 deletions(-) + +diff --git a/postlogin.c b/postlogin.c +index 7c749ef..8a3d9d2 100644 +--- a/postlogin.c ++++ b/postlogin.c +@@ -1788,7 +1788,8 @@ handle_mdtm(struct vsf_session* p_sess) + if (do_write != 0) + { + str_split_char(&p_sess->ftp_arg_str, &s_filename_str, ' '); +- modtime = vsf_sysutil_parse_time(str_getbuf(&p_sess->ftp_arg_str)); ++ modtime = vsf_sysutil_parse_time( ++ str_getbuf(&p_sess->ftp_arg_str), tunable_use_localtime); + str_copy(&p_sess->ftp_arg_str, &s_filename_str); + } + resolve_tilde(&p_sess->ftp_arg_str, p_sess); +@@ -1809,7 +1810,7 @@ handle_mdtm(struct vsf_session* p_sess) + else + { + retval = vsf_sysutil_setmodtime( +- str_getbuf(&p_sess->ftp_arg_str), modtime, tunable_use_localtime); ++ str_getbuf(&p_sess->ftp_arg_str), modtime); + if (retval != 0) + { + vsf_cmdio_write(p_sess, FTP_FILEFAIL, +diff --git a/sysutil.c b/sysutil.c +index e847650..66d4c5e 100644 +--- a/sysutil.c ++++ b/sysutil.c +@@ -2819,11 +2819,13 @@ vsf_sysutil_syslog(const char* p_text, int severe) + } + + long +-vsf_sysutil_parse_time(const char* p_text) ++vsf_sysutil_parse_time(const char* p_text, int is_localtime) + { ++ long res; + struct tm the_time; + unsigned int len = vsf_sysutil_strlen(p_text); + vsf_sysutil_memclr(&the_time, sizeof(the_time)); ++ the_time.tm_isdst = -1; + if (len >= 8) + { + char yr[5]; +@@ -2848,17 +2850,18 @@ vsf_sysutil_parse_time(const char* p_text) + the_time.tm_min = vsf_sysutil_atoi(mins); + the_time.tm_sec = vsf_sysutil_atoi(sec); + } +- return mktime(&the_time); ++ res = mktime(&the_time); ++ if (!is_localtime) ++ { ++ res += the_time.tm_gmtoff; ++ } ++ return res; + } + + int +-vsf_sysutil_setmodtime(const char* p_file, long the_time, int is_localtime) ++vsf_sysutil_setmodtime(const char* p_file, long the_time) + { + struct utimbuf new_times; +- if (!is_localtime) +- { +- the_time -= s_timezone; +- } + vsf_sysutil_memclr(&new_times, sizeof(new_times)); + new_times.actime = the_time; + new_times.modtime = the_time; +diff --git a/sysutil.h b/sysutil.h +index 7a59f13..b90f6ca 100644 +--- a/sysutil.h ++++ b/sysutil.h +@@ -349,9 +349,9 @@ void vsf_sysutil_chroot(const char* p_root_path); + */ + long vsf_sysutil_get_time_sec(void); + long vsf_sysutil_get_time_usec(void); +-long vsf_sysutil_parse_time(const char* p_text); ++long vsf_sysutil_parse_time(const char* p_text, int is_localtime); + void vsf_sysutil_sleep(double seconds); +-int vsf_sysutil_setmodtime(const char* p_file, long the_time, int is_localtime); ++int vsf_sysutil_setmodtime(const char* p_file, long the_time); + + /* Limits */ + void vsf_sysutil_set_address_space_limit(unsigned long bytes); +-- +2.24.1 + diff --git a/0002-Drop-an-unused-global-variable.patch b/0002-Drop-an-unused-global-variable.patch new file mode 100644 index 0000000..53af589 --- /dev/null +++ b/0002-Drop-an-unused-global-variable.patch @@ -0,0 +1,56 @@ +From d0045e35674d64d166d17c3c079ae03e8c2e6361 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= +Date: Thu, 13 Feb 2020 17:29:06 +0100 +Subject: [PATCH 2/2] Drop an unused global variable + +The global variable `s_timezone` is not used anymore, so we can drop +it. +--- + sysutil.c | 17 +++-------------- + 1 file changed, 3 insertions(+), 14 deletions(-) + +diff --git a/sysutil.c b/sysutil.c +index 66d4c5e..0ccf551 100644 +--- a/sysutil.c ++++ b/sysutil.c +@@ -72,8 +72,6 @@ static struct timeval s_current_time; + static int s_current_pid = -1; + /* Exit function */ + static exitfunc_t s_exit_func; +-/* Difference in timezone from GMT in seconds */ +-static long s_timezone; + + /* Our internal signal handling implementation details */ + static struct vsf_sysutil_sig_details +@@ -2661,7 +2659,6 @@ char* vsf_sysutil_get_tz() + void + vsf_sysutil_tzset(void) + { +- int retval; + char *tz=NULL, tzbuf[sizeof("+HHMM!")]; + time_t the_time = time(NULL); + struct tm* p_tm; +@@ -2681,17 +2678,9 @@ vsf_sysutil_tzset(void) + { + die("localtime"); + } +- retval = strftime(tzbuf, sizeof(tzbuf), "%z", p_tm); +- tzbuf[sizeof(tzbuf) - 1] = '\0'; +- if (retval == 5) +- { +- s_timezone = ((tzbuf[1] - '0') * 10 + (tzbuf[2] - '0')) * 60 * 60; +- s_timezone += ((tzbuf[3] - '0') * 10 + (tzbuf[4] - '0')) * 60; +- if (tzbuf[0] == '+') +- { +- s_timezone *= -1; +- } +- } ++ /* Not sure if the following call to strftime() has any desired side ++ effects, so I'm keeping it to be safe. */ ++ (void) strftime(tzbuf, sizeof(tzbuf), "%z", p_tm); + /* Call in to the time subsystem again now that TZ is set, trying to force + * caching of the actual zoneinfo for the timezone. + */ +-- +2.24.1 + diff --git a/vsftpd.spec b/vsftpd.spec index 644221c..f81c2f8 100644 --- a/vsftpd.spec +++ b/vsftpd.spec @@ -2,7 +2,7 @@ Name: vsftpd Version: 3.0.3 -Release: 36%{?dist} +Release: 37%{?dist} Summary: Very Secure Ftp Daemon # OpenSSL link exception @@ -93,6 +93,8 @@ Patch63: 0001-Set-s_uwtmp_inserted-only-after-record-insertion-rem.patch Patch64: 0002-Repeat-pututxline-if-it-fails-with-EINTR.patch Patch65: 0001-Repeat-pututxline-until-it-succeeds-if-it-fails-with.patch Patch66: 0001-Fix-assignment-of-an-enumerator-of-a-different-type.patch +Patch67: 0001-Fix-timestamp-handling-in-MDTM.patch +Patch68: 0002-Drop-an-unused-global-variable.patch %description vsftpd is a Very Secure FTP daemon. It was written completely from @@ -161,6 +163,10 @@ mkdir -p $RPM_BUILD_ROOT/%{_var}/ftp/pub %{_var}/ftp %changelog +* Thu Feb 13 2020 Ondřej Lysoněk - 3.0.3-37 +- Fix timestamp handling in MDTM +- Resolves: rhbz#1567855 + * Fri Feb 07 2020 Ondřej Lysoněk - 3.0.3-36 - Fix build with gcc 10 - Resolves: rhbz#1800239