Thread: RE: [Quickfix-developers] potential infinite loop in QuickFIX Message constructor
Brought to you by:
orenmnero
From: Brendan B. B. <br...@ka...> - 2004-09-28 23:51:33
|
Yihu, You might want to see if my fix in Message::setString(...) for a related problem works as well. The message I sent on 9/2 follows. Regards, Brendan Hi, Suppose a FIX::Message is constructed from a string and suppose the string is a concatenation of two FIX messages (e.g. suppose you're reading from a log of FIX msgs which somehow was corrupted). In Message::setString(const std::string, bool, const DataDictionary *), w/o the two lines I've added below, this will be an infinite loop as pos will increase through the massage which corresponds to the 'first' message and then reset to a low value when it reaches the 'second' message in the string i.e. pos will never be >= string.size(). while ( pos < string.size() ) { FieldBase field = extractField( string, pos, pDataDictionary ); // Begin brendan, 9/2/04 if (pos < pos2) throw InvalidMessage(); else pos2 = pos; // End brendan, 9/2/04 if ( count < 3 && headerOrder[ count++ ] != field.getField() ) if ( doValidation ) throw InvalidMessage(); ... } Regards, Brendan > Message: 2 > Date: Mon, 27 Sep 2004 16:58:18 -0400 > From: Yihu Fang <Yih...@re...> > To: Oren Miller <or...@qu...>, > qui...@li... > Subject: [Quickfix-developers] potential infinite loop in > QuickFIX Message constructor > > This is a multi-part message in MIME format. > > ------_=_NextPart_001_01C4A4D4.B7119FAC > Content-Type: text/plain; charset="us-ascii" > Content-Transfer-Encoding: quoted-printable > > Oren, > > =20 > > There is a bug in QuickFIX Message constructor which may exists in all > versions. An ill-formatted FIX message can let the constructor run into > a tight infinite loop. If the FIX message has an extra white space at > the end of trailer "8=3DFIX.4.0<SOH>...<SOH>10=3Dxxx<SOH> ", or any extra > characters in that matter, the constructor calls setString() and results > in an infinite loop. > > =20 > > The fix is to check the value of the equalSign and throw an exception if > not found. The diff of current CVS Message.cpp should be: > > =20 > > 560a561,562 > > > if (equalSign =3D=3D std::string::npos) > > > throw InvalidMessage(); > > =20 > > Thanks. > > =20 > > -Yihu |
From: Oren M. <or...@qu...> - 2004-09-29 14:15:28
|
Brendan, I wrote a test case for this, but have not been able to duplicate the behavior. Do you have an example of a string that would cause this? --oren On Sep 28, 2004, at 5:51 PM, Brendan B. Boerner wrote: > QuickFIX Documentation: > http://www.quickfixengine.org/quickfix/doc/html/index.html > QuickFIX FAQ: > http://www.quickfixengine.org/wikifix/index.php?QuickFixFAQ > QuickFIX Support: http://www.quickfixengine.org/services.html > > Yihu, > > You might want to see if my fix in Message::setString(...) for a > related problem works as well. > > The message I sent on 9/2 follows. > > Regards, > Brendan > > Hi, > > Suppose a FIX::Message is constructed from a string and suppose the > string is a concatenation of two FIX messages (e.g. suppose you're > reading from a log of FIX msgs which somehow was corrupted). > > In Message::setString(const std::string, bool, const DataDictionary > *), w/o the two lines I've added below, this will be an infinite loop > as pos will increase through the massage which corresponds to the > 'first' message and then reset to a low value when it reaches the > 'second' message in the string i.e. pos will never be >= > string.size(). > > while ( pos < string.size() ) > { > FieldBase field = extractField( string, pos, pDataDictionary ); > // Begin brendan, 9/2/04 > if (pos < pos2) throw InvalidMessage(); > else pos2 = pos; > // End brendan, 9/2/04 > > if ( count < 3 && headerOrder[ count++ ] != field.getField() ) > if ( doValidation ) throw InvalidMessage(); > ... > } > > Regards, > Brendan > > > > >> Message: 2 >> Date: Mon, 27 Sep 2004 16:58:18 -0400 >> From: Yihu Fang <Yih...@re...> >> To: Oren Miller <or...@qu...>, >> qui...@li... >> Subject: [Quickfix-developers] potential infinite loop in >> QuickFIX Message constructor >> >> This is a multi-part message in MIME format. >> >> ------_=_NextPart_001_01C4A4D4.B7119FAC >> Content-Type: text/plain; charset="us-ascii" >> Content-Transfer-Encoding: quoted-printable >> >> Oren, >> >> =20 >> >> There is a bug in QuickFIX Message constructor which may exists in all >> versions. An ill-formatted FIX message can let the constructor run >> into >> a tight infinite loop. If the FIX message has an extra white space at >> the end of trailer "8=3DFIX.4.0<SOH>...<SOH>10=3Dxxx<SOH> ", or any >> extra >> characters in that matter, the constructor calls setString() and >> results >> in an infinite loop. >> >> =20 >> >> The fix is to check the value of the equalSign and throw an exception >> if >> not found. The diff of current CVS Message.cpp should be: >> >> =20 >> >> 560a561,562 >> >>> if (equalSign =3D=3D std::string::npos) >> >>> throw InvalidMessage(); >> >> =20 >> >> Thanks. >> >> =20 >> >> -Yihu > > > ------------------------------------------------------- > This SF.net email is sponsored by: IT Product Guide on > ITManagersJournal > Use IT products in your business? Tell us what you think of them. Give > us > Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out > more > http://productguide.itmanagersjournal.com/guidepromo.tmpl > _______________________________________________ > Quickfix-developers mailing list > Qui...@li... > https://lists.sourceforge.net/lists/listinfo/quickfix-developers > |
From: Brendan B. B. <br...@ka...> - 2004-09-29 15:14:54
|
Oren, The following produces it for me (replace endlines w/^As, the message should end with the 'abc' following tag10=171). The CRC/Lengths won't be accurate as I've changed some of the fields but will repro the problem. This is w/QF v1.8.0. Regards, Brendan 8=FIX.4.2 9=302 35=S 34=5130 49=TARGET 52=20040901-20:44:54 56=SENDER 15=USD 40=C 55=USD/CAD 60=20040901-20:44:54 64=20040902 117=ABC 131=16 132=1.3072 133=1.3076 134=100000.000000 135=100000.000000 167=FOR 188=1.3072 189=0.00 190=1.3076 191=0.00 301=0 647=0.000000 648=0.000000 10=171 abc > -----Original Message----- > From: Oren Miller [mailto:or...@qu...] > Sent: Wednesday, September 29, 2004 8:33 AM > To: br...@ka... > Cc: qui...@li...; Yih...@re... > Subject: Re: [Quickfix-developers] potential infinite loop in QuickFIX > Message constructor > > > Brendan, > > I wrote a test case for this, but have not been able to duplicate the > behavior. Do you have an example of a string that would cause this? |
From: Oren M. <or...@qu...> - 2004-09-29 15:59:15
|
Yeah, in that case Yihu's patch covers this case as well. --oren On Sep 29, 2004, at 10:14 AM, Brendan B. Boerner wrote: > Oren, > > The following produces it for me (replace endlines w/^As, the message > should end with the 'abc' following tag10=171). > > The CRC/Lengths won't be accurate as I've changed some of the fields > but will repro the problem. > > This is w/QF v1.8.0. > > Regards, > Brendan > > 8=FIX.4.2 > 9=302 > 35=S > 34=5130 > 49=TARGET > 52=20040901-20:44:54 > 56=SENDER > 15=USD > 40=C > 55=USD/CAD > 60=20040901-20:44:54 > 64=20040902 > 117=ABC > 131=16 > 132=1.3072 > 133=1.3076 > 134=100000.000000 > 135=100000.000000 > 167=FOR > 188=1.3072 > 189=0.00 > 190=1.3076 > 191=0.00 > 301=0 > 647=0.000000 > 648=0.000000 > 10=171 > abc > > > >> -----Original Message----- >> From: Oren Miller [mailto:or...@qu...] >> Sent: Wednesday, September 29, 2004 8:33 AM >> To: br...@ka... >> Cc: qui...@li...; Yih...@re... >> Subject: Re: [Quickfix-developers] potential infinite loop in QuickFIX >> Message constructor >> >> >> Brendan, >> >> I wrote a test case for this, but have not been able to duplicate the >> behavior. Do you have an example of a string that would cause this? > > |