Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: [PATCH] Cut down buffer size in ftp_anon_connect()
From: Kris Katterjohn <kjak () ispwest com>
Date: Wed, 08 Mar 2006 08:19:18 -0600

Matthew Murphy wrote:
Kris Katterjohn wrote:
The attached patch cuts down the size of the 'command' buffer in
ftp_anon-connect() from 512 to 270. ftp->user can hold 64 bytes and ftp->pass
can hold 256, so 270 will hold "PASS [ftp->pass]\r\n" with a few extra bytes
in there. It also uses sizeof in snprintf() instead of just a number.

Thanks,
Kris Katterjohn

I have two potential concerns about this patch.  First of all, I'd
recommend applying it with parenthesis around sizeof expressions, as in:

    sizeof(command) - 1

rather than:

    sizeof command - 1

Most compilers are tolerant of both forms, but I'm more used to seeing
the latter, and it is (IMO) cleaner.


I don't know what you mean by "tolerant" of both forms. sizeof only requires
the parentheses when it's taking the size of a data type:

sizeof(int)

Parentheses aren't required when it's a variable (like in the patch). Not that
I have a problem with the parentheses in this situation, just clearing things
up :)

Further, in looking at your choice of buffer size, I'm wondering why 270
instead of 264?

PASS [256 chars]\r\n\0

is exactly 256+8 = 264.  Choosing 264 also makes the buffer eight-byte
aligned, which may save a few bytes of stack space (in addition to the
six byte slack) or make the binary a (microscopic) hair faster by
avoiding alignment issues.


No real reason, just that when the size of the user/pass buffers are in
another file (and not RIGHT there to be seen) and the string *could* possibly
change (not that I foresee that anytime soon), I sometimes give it a few extra
bytes.

264 sounds okay to me, though.


Thanks,
Kris


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


  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]
AlienVault