Hi again, Not sure why I didn't catch this before, but the nextmods variable is also vulnerable to the same overflow condition. A more complete fix is:
*316:** *char nextpeptide[*1000*]; ... *324:** *char nextmods[*1000*]; ... *440: *int result = sscanf(nextline, "%d %lf %lf %d %*1000*s %d %*1000*s %lf %s %d %d;%c", *441: *&first, &mass, &massdiff, &nextionmatch, nextpeptide, &first, nextmods, &nextionscore, discard, &first, &first, &nextc); Best wishes, Phil On Wednesday, 16 January 2013 19:42:36 UTC, Luis wrote: > > Hi Phil, > Many thanks for finding, fixing, and reporting this bug. We'll > incorporate the fix into our next release. > Cheers, > --Luis > > > On Wed, Jan 16, 2013 at 7:35 AM, Phil Charles <[email protected]<javascript:> > > wrote: > >> Hi guys, >> >> I ran across a bug in >> Parsers/Algorithm2XML/Mascot2XML/MascotConverter.cxx while trying to work >> out why Mascot2XML was segfaulting on a particular Mascot dat file. >> >> On line 316 the 'nextpeptide' variable is defined to hold peptide >> sequences extracted from the dat file. It has a length of 128. >> *316: *char nextpeptide[128]; >> >> Then, on line 440, a value is assigned to nextpeptide, among other >> things, by sscanf (5th item, %s): >> *440: *int result = sscanf(nextline, "%d %lf %lf %d %s %d %s %lf %s %d >> %d;%c", >> *441: *&first, &mass, &massdiff, &nextionmatch, nextpeptide, &first, >> nextmods, &nextionscore, discard, &first, &first, &nextc); >> >> However, sscanf isn't overflow safe, so if the peptide is more than 128 >> residues (unlikely, yes, but not impossible, and it only needs one...) the >> remaining characters will overflow nextpeptide and overwrite whatever's >> next - on my build, this meant it overwrote the variable holding the MIME >> delimiter used to parse the dat file, which led to a later segfault as the >> spectra_ array wasn't populated. Incidentally, compiling with debug flags >> using 'make debug' seemed to solve the error, or at least prevent the >> segfault - not quite sure why this was (maybe it overflowed into something >> less critical), but it certainly made tracking down the issue a bit harder. >> >> The fix I applied was to increase the nextpeptide length and >> overflow-proof the sscanf with respect to this variable (although if you >> have a 1000 residue there's probably something else gone wrong!) with a >> length limit on that %s. >> *316:** *char nextpeptide[*1000*]; >> ... >> *440: *int result = sscanf(nextline, "%d %lf %lf %d %*1000*s %d %s %lf >> %s %d %d;%c", >> *441: *&first, &mass, &massdiff, &nextionmatch, nextpeptide, &first, >> nextmods, &nextionscore, discard, &first, &first, &nextc); >> >> Cheers, >> >> Phil >> >> -- >> You received this message because you are subscribed to the Google Groups >> "spctools-discuss" group. >> To view this discussion on the web visit >> https://groups.google.com/d/msg/spctools-discuss/-/9LUaN1h-4mAJ. >> To post to this group, send email to >> [email protected]<javascript:> >> . >> To unsubscribe from this group, send email to >> [email protected] <javascript:>. >> For more options, visit this group at >> http://groups.google.com/group/spctools-discuss?hl=en. >> > > -- You received this message because you are subscribed to the Google Groups "spctools-discuss" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/spctools-discuss?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
