Re: [Patch] Service Fingerprints in XML

[Brandon Enright <bmenrigh () ucsd edu>]

On Tue, 2006-04-18 at 00:16 -0700, Fyodor wrote:
> Hi Brandon.  Thanks for the patch.  It looks pretty good and I'd like
> to add it, but I have a couple concerns:
>
> > +  if (sd->service_fp) {
> > +    xml_servicefp = xml_sf_convert(xml_convert(sd->service_fp));
> > +    versionxmlstring += " servicefp=\"";
> > +    versionxmlstring += xml_servicefp;
> > +    free(xml_devicetype); xml_servicefp = NULL;
> > +    versionxmlstring += '\"';
> > +  }
>
> Do you mean to free(xml_servicefp) here instead of xml_devicetype?
>
> > +/* Remove all "\nSF:" from fingerprints */
> > +char* xml_sf_convert (const char* str) {
>
> I think there is room to improve this implementation.  It copies the
> string a whole lot of times, and has some variables (such as substr[]
> which don't seem necessary).  Couldn't you do something like:
>
> char *temp = (char *) safe_malloc(strlen(str) + 1);
> char *dst = temp, *src=str;
> while(*src) {
>    if (strcmp(src, "\nSF:") == 0) {
>      src += 4;
>    }
>    if (*src)
>      *dst++ = *src++;
> }
> *dst = '\0';
>
> Or am I missing some reason why this won't work?  Do you think you
> could make and test these changes and then resend your patch?
>
> Thanks,
> Fyodor

Okay I've attached a much cleaner version of this patch.  I used you're
version of xml_sf_convert as a base but capped fingerprints to 2048
bytes.  2048 bytes should be good enough for all but the largest
fingerprints.

I'm pretty happy with this patch now, let me know what you think.

Brandon

Attachment: nmap.dtd.patch
Description:

Attachment: output.cc.patch
Description:

Attachment: output.h.patch
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev