>>>>> On Tue, 22 Dec 2009 09:42:37 +0100, Magnus Fromreide >>>>> <[email protected]> said:
>> Should we really keep a single function >> read_config_read_octet_string() that is used for both reading binary >> data and reading ASCII strings ? Maybe it's better to have two >> functions, one for reading binary data and one for reading ASCII >> strings. MF> I think you are right, it should be two functions. When the function was written (I think maybe by me) the assumptions were: 1) Null terminating a binary string if the length didn't include the null termination isn't a bad thing; in fact it's a "safe" thing even for binary data. 2) Saving to hex/binary in a lot of cases where the data *looks* like it's ascii but may be binary is also a safe thing. Not worrying about bad-user-data-insertion is also safe. Consider someone sending an snmpset of "foo\nrwuser joe\n" to the agent and having the agent store it in ascii. Storing strings as binary strings even when the content is generally ascii is again: safer. 3) Expectations that bad coders exist; IE, ones who would ignore the length field of certain data attributes and print a buffer as a null-terminated string even though it was never intended to be. This happens *a lot* unfortunately with data that *looks* like it's probably safe to print. Consider one of the more interesting cases: session->securityName Now, this char * field is an interesting case for a few reasons (note this is a generic example of a securityName and isn't stored directly via read_config but many other securityName fields are that are copied to and from this one). A) It has a securityNameLen field associated with it, but the securityName is frequently used by both our code (and I'm more than sure external 3rd-party code) such that it's printed straight out and ignores the securityNameLen field and is assumed to be NULL terminated. Why not, securityNames are ascii right? Why wouldn't it be NULL terminated. B) And that's where securityNames get even more interesting. If you go read the SNMPv3 RFCs you'll find that they must be human-ascii strings once they pass through the security model. In the case of USM, USM mandates that they're always ascii so the transformation is 1:1. But the SNMPv3 architecture allows for securityNames to be binary up until the point they reach the security model. (Now, admittedly, the session should probably be pointing to the ascii version). My point isn't that this will directly affect read-config stuff, but it's an important point when considering future expansion and changes: assuming binary but being safe by always null-terminating is, IMHO, a good thing. We can't fix everyone else's code. I'm very proud to say that *we* have very good coders and though we make mistakes, our knowledge of C is quite outstanding. I've actually worked with a lot of other 3rd-party companies who don't have the grasp of the safer behavior like our coders generally do. Unfortunately. And no, I don't consider a policy of "that's there own fault" acceptable. It's our job to make things easy for them if we want a good, robust and successful package that don't cause third parties to end up with CVEs. Um, ok, that was a rant. Sorry ;-) The important end point from all of this is I have yet to hear why this is a bad thing? a) read 0x656667 from the .conf file b) pad it to 0x65666700 c) return it with a length of 3 Where is the evil? What is breaking because of that? What would the benefits of removing that 00 be? I see no gains for removing it and many positives. -- Wes Hardaker Please mail all replies to [email protected] ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev _______________________________________________ Net-snmp-coders mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
