Re: [Patch] Service Fingerprints in XML

[Fyodor <fyodor () insecure org>]

On Tue, Apr 18, 2006 at 05:53:32AM +0000, Brandon Enright wrote:
> I find the service fingerprint submission information that Nmap
> sometimes spits out to be very useful in finding back-door services on
> compromised hosts.  Unfortunately this information isn't available in
> the XML output.  Similar to Joshua Abraham's patch, this patch adds the
> service fingerprint information to the XML output whenever the
> fingerprints goes to STDOUT.  The information is added as an attribute
> called servicefp to the service element.

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


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