error when using TimeBase with Time Zone other than ZULU

RESOLVED FIXED in 4.2.5

Status

JSS
Library
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Bartlomiej Lopatka, Assigned: glen beasley)

Tracking

unspecified
4.2.5
x86
Windows XP

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1) Gecko/20061010 Firefox/2.0

When trying to parse UTCTime or Generalized time in format: yyyyMMddhhmmss.msTZ
where TZ is TimeZone in format +hhmm or -hhmm the code throws ArrayIndexOutOfBoundsException. Error is in line 259: http://lxr.mozilla.org/mozilla/source/security/jss/org/mozilla/jss/asn1/TimeBase.java#259
The code looks like: 
if( chars[i] == '+' || chars[i] == '-') {
254                     checkBounds(i+1, 4, chars.length);
255                     hourOff = (chars[i+1] - '0') * 10;
256                     hourOff += chars[i+2] - '0';
257                     minOff = (chars[i+3] - '0') * 10;
258                     minOff += chars[i+4] - '0';
259                     i += 5;
260                     checkRange(hourOff, 0, 23, "hour offset");
261                     checkRange(minOff, 0, 59, "minute offset");
262                     if( chars[i] == '-' ) {
263                         hourOff = -hourOff;
264                         minOff = -minOff;
265                     }
266                     tz = (TimeZone) TimeZone.getTimeZone("GMT").clone();
267                     tz.setRawOffset( ((hourOff*60)+minOff)*60*1000 );

This code is wrong, because it can't check the chars[i]=='-' after the i+=5 operation.
Code should be something like that:
if( chars[i] == '+' || chars[i] == '-') {
254                     checkBounds(i+1, 4, chars.length);
255                     hourOff = (chars[i+1] - '0') * 10;
256                     hourOff += chars[i+2] - '0';
257                     minOff = (chars[i+3] - '0') * 10;
258                     minOff += chars[i+4] - '0';
259                     //changes here
                        //i += 5;   this line is removed from her
260                     checkRange(hourOff, 0, 23, "hour offset");
261                     checkRange(minOff, 0, 59, "minute offset");
262                     if( chars[i] == '-' ) {
263                         hourOff = -hourOff;
264                         minOff = -minOff;
265                     }
                        //new place for our wrong line :)
                        i += 5;
266                     tz = (TimeZone) TimeZone.getTimeZone("GMT").clone();
267                     tz.setRawOffset( ((hourOff*60)+minOff)*60*1000 );

I hope, that my report is useful, and somebody will revise this code.
Cheers
Bartlomiej Lopatka

Reproducible: Always

Steps to Reproduce:
1.Use the code :) I'm using this library in my own code, so I can't tell you how to reproduce this.
2.
3.

Actual Results:  
my program crashes

Expected Results:  
it should interpreted my date :)
(Reporter)

Comment 1

12 years ago
Created attachment 247957 [details]
source withe removed bug
(Assignee)

Comment 2

12 years ago
Created attachment 256126 [details] [diff] [review]
fix for ArrayIndexOutOfBoundsException

Thanks Bartlomiej for the fix. 
next time if you attached the fix by doing a 
cvs update
then 
cvs diff -u6 > patch
then attach the patch.
Attachment #247957 - Attachment is obsolete: true
Attachment #256126 - Flags: superreview?(neil.williams)
Attachment #256126 - Flags: review?(glen.beasley)
(Assignee)

Updated

12 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → 4.2.5
(Assignee)

Updated

12 years ago
Attachment #256126 - Flags: review?(glen.beasley) → review+

Updated

12 years ago
Attachment #256126 - Flags: superreview?(neil.williams) → superreview+
(Assignee)

Comment 3

12 years ago
cvs commit -m "363103 fix by Bartlomiej Lopatka  r=glen sr=neil"
cvs commit: Examining .
Enter passphrase for key '/Users/b/.ssh/id_dsa': 
Checking in TimeBase.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/asn1/TimeBase.java,v  <--  TimeBase.java
new revision: 1.5; previous revision: 1.4
done
 
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.