Bug#319489: Buffer overflow in Description parsing
On Fri, Jul 22, 2005 at 10:28:10AM -0400, Anthony DeRobertis wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
In helpers.cpp, we find this code, which parses data returned from ebay:
/*
* Parse the description out of the buffer first. This is
* most easily done at the buffer-level and not as we try
* to read the buffer in a line-oriented manner. There is
* probably a need to re-write this parser all together,
* but that's not what I'm going to do right now.
* Thanks to Bob Beaty!
*/
scratch = strstr(Buff, ) -);
if (scratch != NULL) {
// move past the ) -
scratch += 3;
// move past any whitespace
while (isspace(*scratch)) scratch++;
// copy over the description to a newline
idx = 0;
while (*scratch != '\n') {
Description[idx++] = *scratch++;
}
// NULL terminate the description I just parsed off
Description[idx] = '\0';
} else {
return FALSE;
}
Notice how it copies an abitrary amount of data, as much as ebay returns
before \n, into Description.
This should work for now.
Index: helpers.cpp
===
RCS file: /cvsroot/bidwatcher/bidwatcher/Attic/helpers.cpp,v
retrieving revision 1.90.2.58
diff -u -r1.90.2.58 helpers.cpp
--- helpers.cpp 15 May 2005 21:53:00 - 1.90.2.58
+++ helpers.cpp 23 Jul 2005 13:45:32 -
@@ -1043,7 +1043,7 @@
}
}
- int idx=0;
+ unsigned idx=0;
int cnt;
int ended_early=0;
int auc_type=TYPE_EBAY;
@@ -1086,7 +1086,7 @@
while (isspace(*scratch)) scratch++;
// copy over the description to a newline
idx = 0;
- while (*scratch != '\n') {
+ while (*scratch != '\n' idx sizeof Description) {
Description[idx++] = *scratch++;
}
// NULL terminate the description I just parsed off
The rest of this struct looks disturbing, too.
Welcome to inheriting a large chunk of code written by people who aren't
around anymore. ;-)
I actually did go around a long while ago and fix a lot of these types
of things, but that one didn't jump out at me. Thanks for bringing it
up.
-kpd
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#319489: Buffer overflow in Description parsing
Kevin Dwyer wrote:
- while (*scratch != '\n') {
+ while (*scratch != '\n' idx sizeof Description) {
I strongly suspect that should be sizeof(Description)-1 because you're
going to NULL-terminate... (didn't go back and look at the code to check
closely)
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#319489: Buffer overflow in Description parsing
On Sat, Jul 23, 2005 at 01:56:00PM -0400, Anthony DeRobertis wrote:
Kevin Dwyer wrote:
- while (*scratch != '\n') {
+ while (*scratch != '\n' idx sizeof Description) {
I strongly suspect that should be sizeof(Description)-1 because you're
going to NULL-terminate... (didn't go back and look at the code to check
closely)
Correct. I just glanced at it and didn't test/commit that change.
-kpd
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#319489: Buffer overflow in Description parsing
Package: bidwatcher
Version: 1.3.17-1
Severity: grave
Tags: security
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
In helpers.cpp, we find this code, which parses data returned from ebay:
/*
* Parse the description out of the buffer first. This is
* most easily done at the buffer-level and not as we try
* to read the buffer in a line-oriented manner. There is
* probably a need to re-write this parser all together,
* but that's not what I'm going to do right now.
* Thanks to Bob Beaty!
*/
scratch = strstr(Buff, ) -);
if (scratch != NULL) {
// move past the ) -
scratch += 3;
// move past any whitespace
while (isspace(*scratch)) scratch++;
// copy over the description to a newline
idx = 0;
while (*scratch != '\n') {
Description[idx++] = *scratch++;
}
// NULL terminate the description I just parsed off
Description[idx] = '\0';
} else {
return FALSE;
}
Notice how it copies an abitrary amount of data, as much as ebay returns
before \n, into Description.
In bidwatcher.h, Description is defined as a char array:
struct auctioninfo {
unsigned long long ItemNumber; /* Item Number (User Entered) */
char Description[129];/* Description Of Item For sale */
char Comments[COMMENT_LENGTH];/* User-defined comments*/
// ...
}
The rest of this struct looks disturbing, too.
- -- System Information:
Debian Release: testing/unstable
APT prefers testing
APT policy: (500, 'testing'), (500, 'stable'), (130, 'unstable'), (120,
'experimental')
Architecture: i386 (i686)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.10-bohr
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Versions of packages bidwatcher depends on:
ii libc6 2.3.2.ds1-22 GNU C Library: Shared libraries an
ii libcurl3 7.14.0-2 Multi-protocol file transfer libra
ii libgcc11:4.0.0-9 GCC support library
ii libglib1.2 1.2.10-10 The GLib library of C routines
ii libgtk1.2 1.2.10-17 The GIMP Toolkit set of widgets fo
ii libidn11 0.5.13-1.0GNU libidn library, implementation
ii libssl0.9.70.9.7e-3 SSL shared libraries
ii libstdc++5 1:3.3.5-13The GNU Standard C++ Library v3
ii libx11-6 4.3.0.dfsg.1-14 X Window System protocol client li
ii libxext6 4.3.0.dfsg.1-14 X Window System miscellaneous exte
ii libxi6 4.3.0.dfsg.1-14 X Window System Input extension li
ii xlibs 4.3.0.dfsg.1-14 X Keyboard Extension (XKB) configu
ii zlib1g 1:1.2.2-4.sarge.1 compression library - runtime
bidwatcher recommends no packages.
- -- no debconf information
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQFC4QJ6+z+IwlXqWf4RAmWmAJkBIdsx9WRAK5b+hwJv+6m2zKFoVACeMi2o
BQ8aodXcS4CfbH8/FRjNK2M=
=AhfB
-END PGP SIGNATURE-
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
