[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Proposed update: gabedit/2.4.2-2+wheezy1 fixing #703965

Hi Adam,

Am Montag, den 29.07.2013, 21:45 +0100 schrieb Adam D. Barratt:
> On Mon, 2013-07-29 at 00:44 +0200, Daniel Leidert wrote:
> > I'm proposing a fix for gabedit in Wheezy. A buffer overflow has been
> > detected, which can be fixed with a one-liner. See these references:
> 
> Thanks for caring about fixing bugs in stable. For future reference,
> it's generally easier (at least for us) to track such requests if
> they're filed in the BTS (either via reportbug, or separately with the
> appropriate usertags).
> 
> +gabedit (2.4.2-2+wheezy1) stable; urgency=low
> 
> 2.4.2-2+deb7u1, please.

No problem.

> +--- a/src/Display/AnimationGeomConv.c
> ++++ b/src/Display/AnimationGeomConv.c
> +@@ -1441,6 +1441,7 @@
> +                       if (l==2) AtomCoord[0][1]=tolower(AtomCoord[0][1]);
> + 
> + 
> ++                      sprintf(AtomCoord[0],"%s",get_symbol_using_z(atoi(dum)));
> +                       sprintf(listOfAtoms[j].symbol,"%s",AtomCoord[0]);
> 
> Apologies if I'm missing something, but doesn't that sprintf() call
> overwrite all of the manipulation performed on AtomCoord[0] (or its
> components) during the previous few lines?

Please find attached an explanation by the upstream author.

Regards, Daniel
--- Begin Message --- 
Dear Daniel,
In the old version (without  sprintf(AtomCoord[0],"%s",get_symbol_using_z(atoi(dum))); ) :
To define the symbol of atoms, Gabedit used the first column of the geometry section of Gamess output file.
This column contain "generally" the symbol of atoms. In this case, no problem (no overflow)
However , the users of Gamess can also use  the name of atoms (Carbon, Oxygen,...). In this case the length of string AtomCoord[0] can be greater 3.
The length of listOfAtoms[j].symbol table is limited to 4 . sprintf(listOfAtoms[j].symbol,"%s",AtomCoord[0]); produce a overflow !

In the new version (with  sprintf(AtomCoord[0],"%s",get_symbol_using_z(atoi(dum))); ) :
To define the symbol of atoms, Gabedit use the second column of the geometry section of Gamess output file. The integer of this column contain
the z (the number of electrons in the atom). AtomCoord[0] will be a string of 3 characters. There is no overflow in this case.

Certainly I could fix the bug by others (elegant)  methods :
for example, I can remove 
                       if (l==2) AtomCoord[0][1]=tolower(AtomCoord[0][1]);
                       sprintf(listOfAtoms[j].symbol,"%s",AtomCoord[0]);

and add sprintf(listOfAtoms[j].symbol,"%s",get_symbol_using_z(atoi(dum))););
 
Best regards,

________________________________________
De : Daniel Leidert [daniel.leidert@wgdd.de]
Date d'envoi : samedi 3 août 2013 14:01
À : allouchear@users.sourceforge.net
Objet : [Fwd: Re: Proposed update: gabedit/2.4.2-2+wheezy1 fixing #703965]

Hi,

I need an explanation about the fix you applied in gabedit 2.4.7 for a
buffer overflow reported by a Debian user. I have to answer a question
by our Debian FTP masters about the one-line-fix you propose ... see
below. Can you explain the fix a bit further, please?

Regards, Daniel

-------- Weitergeleitete Nachricht --------
> Von: Adam D. Barratt <adam@adam-barratt.org.uk>
> An: Daniel Leidert <daniel.leidert@wgdd.de>
> Kopie: debian-release@lists.debian.org
> Betreff: Re: Proposed update: gabedit/2.4.2-2+wheezy1 fixing #703965
> Datum: Mon, 29 Jul 2013 21:45:43 +0100
>
> Hi,
>
> On Mon, 2013-07-29 at 00:44 +0200, Daniel Leidert wrote:
> > I'm proposing a fix for gabedit in Wheezy. A buffer overflow has been
> > detected, which can be fixed with a one-liner. See these references:
[snip]
> +--- a/src/Display/AnimationGeomConv.c
> ++++ b/src/Display/AnimationGeomConv.c
> +@@ -1441,6 +1441,7 @@
> +                       if (l==2) AtomCoord[0][1]=tolower(AtomCoord[0][1]);
> +
> +
> ++                      sprintf(AtomCoord[0],"%s",get_symbol_using_z(atoi(dum)));
> +                       sprintf(listOfAtoms[j].symbol,"%s",AtomCoord[0]);
>
> Apologies if I'm missing something, but doesn't that sprintf() call
> overwrite all of the manipulation performed on AtomCoord[0] (or its
> components) during the previous few lines?
>
> Regards,
>
> Adam
>
>

--- End Message ---
Reply to: