346091 - editparams.cgi should validate the 'timezone' parameter

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Richard Hurt]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

We are running Bugzilla Version 2.20.1 internally and are getting lots of the following entries in our error log:

[Thu Jul 27 08:44:35 2006] buglist.cgi: Use of uninitialized value in addition (+) at /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm line 118.
[Thu Jul 27 08:44:36 2006] buglist.cgi: Use of uninitialized value in addition (+) at /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm line 118.
[Thu Jul 27 08:44:36 2006] buglist.cgi: Use of uninitialized value in addition (+) at /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm line 118.
[Thu Jul 27 08:44:37 2006] buglist.cgi: Use of uninitialized value in addition (+) at /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm line 118.
[Thu Jul 27 08:44:37 2006] buglist.cgi: Use of uninitialized value in addition (+) at /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm line 118.

This is similar to bug #281920 but instead of Date::Time problems I am seeing Date::Format problems.  The error occurs in the time2str function of the Format module on the following line:

   $time += tz_offset($tzname);


Reproducible: Always

Steps to Reproduce:
1. Hit the buglist.cgi URL
2. Tail the Apache error log file
3. Watch error scroll up



Expected Results:  
I would expect there to not be an error produced.

bash-2.05a$ perl -V
Summary of my perl5 (revision 5 version 8 subversion 7) configuration:
  Platform:
    osname=aix, osvers=5.3.0.0, archname=aix
    uname='aix hctunx58 3 5 00c4b39e4c00 unknown unknown aix '
    config_args='-de -Dcc=gcc -D_ALL_SOURCE -D_ANSI_C_SOURCE -D_POSIX_SOURCE -DUSE_NATIVE_DLOPE -D_LARGE_FILES'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_ALL_SOURCE -D_ANSI_C_SOURCE -D_POSIX_SOURCE -DUSE_NATIVE_DLOPEN -fno-strict-aliasing -pipe -D_LARGE_FILES',
    optimize='-O',
    cppflags='-D_ALL_SOURCE -D_ANSI_C_SOURCE -D_POSIX_SOURCE -DUSE_NATIVE_DLOPEN -fno-strict-aliasing -pipe'
    ccversion='', gccversion='3.3.2', gccosandvers='aix5.3.0.0'
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -Wl,-brtl -Wl,-bdynamic -Wl,-bmaxdata:0x80000000 -L/usr/local/lib -Wl,-b32'
    libpth=/usr/local/lib /lib /usr/lib /usr/ccs/lib
    libs=-lbind -lnsl -lgdbm -ldbm -ldb -ldl -lld -lm -lcrypt -lc -lbsd
    perllibs=-lbind -lnsl -ldl -lld -lm -lcrypt -lc -lbsd
    libc=/lib/libc.a, so=a, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_aix.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Xlinker -bE:/usr/local/lib/perl5/5.8.7/aix/CORE/perl.exp'
    cccdlflags=' ', lddlflags='  -Wl,-bhalt:4 -Wl,-bexpall -Wl,-G -Wl,-bnoentry -lc -L/usr/local/lib'


Characteristics of this binary (from libperl):
  Compile-time options: USE_LARGE_FILES
  Built under aix
  Compiled at Jan 18 2006 08:25:38
  %ENV:
    PERL5LIB="/apps/local/lib/perl5/site_perl"
  @INC:
    /apps/local/lib/perl5/site_perl/5.8.7/aix
    /apps/local/lib/perl5/site_perl/5.8.7
    /apps/local/lib/perl5/site_perl
    /usr/local/lib/perl5/5.8.7/aix
    /usr/local/lib/perl5/5.8.7
    /usr/local/lib/perl5/site_perl/5.8.7/aix
    /usr/local/lib/perl5/site_perl/5.8.7
    /usr/local/lib/perl5/site_perl
    .


#### /usr/local/lib/perl5/site_perl/5.8.7/Date/Format.pm ####
...snip...
sub time2str
{
 my($pkg,$fmt,$time);

 ($pkg,$fmt,$time,$tzname) = @_;

 my $me = ref($pkg) ? $pkg : bless [], $pkg;

 $epoch = $time;

 if(defined $tzname)
  {
   $tzname = uc $tzname;

   $tzname = sprintf("%+05d",$tzname)
        unless($tzname =~ /\D/);

   $time += tz_offset($tzname);
   @$me = gmtime($time);
  }
 else
  {
   @$me = localtime($time);
  }
 $me->[9] = $time;
 _subs($me,$fmt);
}
...snip...

Comment 1

[Frédéric Buclin]

This error appears only when the 'timezone' parameter entered by an administrator is not recognized by Time::Zone, in which case tz_offset() returns an undefined value. You can see the list of valid timezones in Time::Zone (look at the @dstZone and @Zone arrays).

What we should do is to validate the timezone value entered by the administrator and complain if it's not a valid one. I'm morphing the bug summary to reflect that.

Comment 2

[Frédéric Buclin]

Note that the fix is to call Time::Zone::tz_offset() from Bugzilla::Config::Common::check_timezone() and reject the value entered by the admin if 'undef' is returned.

Comment 3

[Richard Hurt]

I can confirm this bug.  My timezone parameter was set to "ET" and when I changed it to "est" the error went away.  I then changed it to "YOMAMA" and the error came back.

Thanx!
  Richard

Comment 4

[victory <[e-mail]>]

Attachment: patch for tip

Comment 5

[Frédéric Buclin]

Comment on attachment 245539 [details] [diff] [review]
patch for tip

>Index: Bugzilla/Config/Common.pm

>+sub check_timezone {
>+    use Time::Zone;

|use Time::Zone| should be at the beginning of the file, not here.


>+    my ($tz) = (@_);

Nit: |my $tz = shift;| or |my ($tz) = @_;|

r=LpSolit

Comment 6

[Frédéric Buclin]

Checking in Bugzilla/Config/Common.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Common.pm,v  <--  Common.pm
new revision: 1.12; previous revision: 1.11
done
Checking in Bugzilla/Config/Core.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Core.pm,v  <--  Core.pm
new revision: 1.6; previous revision: 1.5
done