Bug 62767 (CVE-2004-0645)

<!-- SBZ_reproduce  -->
-

ping 

Created attachment 26006 [details]
wv-1.0.0-fix_overflow.patch

Gentoo Linux patch. I am applying it to abiword and wv packages.

Patch submitted for:

abiword in stable-all, sles9-sld, PLUS, 9.2-all, 9.1-all, 8.2-all

abiword2 in 9.0-all

wv in stable-all, 9.2-all, 9.1-all, 9.0-all

Needs further research: abiword in 8.1-all

abiword was not on NLD. 
 
I created patchinfos for abiword and abiword2. 
 
I still include 8.1 ... please tell me if I should remove it. 

Created attachment 26014 [details]
wv-0.7.2+-fix_overflow.patch

Suggested patch for 8.1. Please review. Version in this abiword is declared as
0.7.2 (which does not need the patch), but in reality it is different from
0.7.2.

In wv-0.7.2, the whole patch can be -R applied, but here not.

abiword package for 8.1 submitted.

is there any check for consumed in this patch at all?  
 
you just set and increment it, but never check it. 

This is in >8.1, but not 8.1. Fixing.

/* the '11' is the max width of an integer (10 digits for '4 billion') + nul */
    while (*token && (consumed < (TIMESTR_SIZE - 11)))

Created attachment 26021 [details]
wv-0.7.2+-fix_overflow.patch

Second version of patch for 8.1.

patch looks good. 

Submitted 8.1 with new patch. Reassigning.

Patchinfos?

already in done/PATCHINFO 
 
-rw-r--r--  1 meissner suse 457 2004-11-15 
18:02 /work/src/done/PATCHINFO/abiword2.patch.box 
-rw-r--r--  1 meissner suse 492 2004-11-15 
18:02 /work/src/done/PATCHINFO/abiword.patch.box 
 
? 

Please create also patchinfo for wv, which exists as stand-alone package, too.
The same bug, the same patch.

We have also wv2 package, but it seems to be code rewrite.

? we already released a security update for wv and wv2 some monmtghs ago to 
fix this issue.... didn't we? 

suse bugzilla #45094 

Bug 60094 was about wv-0.7.2 and fixed for 8.1 and 8.2, now the patch was
applied for stable-all, 9.2-all, 9.1-all, 9.0-all (CAN-2004-0645: wv library
(wvWare) 0.7.4 through 0.7.6 and 1.0.0).

Patchinfos for wv?

will do. 

Created attachment 26305 [details]
wv.patch.box

Packages checked in.

Hmm, on the other hand, are you really sure that this is correct?

@@ -197,6 +202,7 @@
                  case 1:
                      consumed += sprintf (temp, "%d", current->tm_hour);
                      strcat (timestr, temp);
+                     consumed += strlen (temp);
                      break;
                  default:
                      strcat (timestr, "%H");
@@ -212,6 +218,7 @@
                  case 1:
                      consumed += sprintf (temp, "%d", current->tm_min);
                      strcat (timestr, temp);
+                     consumed += strlen (temp);
                      break;
                  default:
                      strcat (timestr, "%M");

It seems to me that the length is added twice to consumed...

Adding a check to verify the return value of sprintf() will also avoid 
'consumed' to be decremented in the case of an error. 

What does that mean? Shouldn't the patch above remove consumed += from the
sprintf line then?

And isn't strlen(temp) undefined if sprintf returns -1?

This looks like the same patch as in bug 60094  
The result of the discussion there was that a half of the patch is wrong and 
the rest is not necessary. 

Hmm, what now? Will you resubmit wv with a fixed patch?

IMHO the only problem that might need fixing is the negative return 
from sprintf. Can it happen? 
 

Just in the case if 'temp' is NULL or something is wrong with 'current->tm_*' 
AFAIK. 
 
 

Temp is declared as this: 
char temp[64]; 
 
current->tm_* comes from localtime and according to man page it is  
in range 0 to 59 or similar. 
 
IMHO wv is good as is, without any patch. 
 
 

the slightly off byte count (always larger) can be ignored. 
 
please proceed as planned. 

Vladimir, I think you're right. I don't know why the secteam wrote that patch,
it is 1) broken and 2) fixes nothing. I guess I shouldn't have checked in those
wv packages. Should I revert them to the original version?

This is a Gentoo secteam work.

i reviewed the patch for wv together with mls, it does not change behaviour at 
all. The "%d" strings all print values of months or hours, which are either 1 
or 2 characters, the rest changes are not needed at all. 
 
mls reverted the "fixes" in autobuild. 
 
I have approved the patchinfos for abiword and abiword2. 

But all three packages (abiword1, abiword2 and wv) use the same patch. Only
abiword in 8.1-all has different one. Is the code in abiword* sources different?

*sigh* 
 
Yes, I looked through all the abiwords we had and this is the case, 
only 8.1 has real problems. 

CVE-2004-0645: CVSS v2 Base Score: 10.0 (AV:N/AC:L/Au:N/C:C/I:C/A:C)