OK, here's the deal. I missed the SQL injection vulnerability until I was applying the patch, by which point it didn't seem to matter. However, this is not a huge risk since the main piece of user-supplied data that is used in the SQL queries is the username, and that gets passed through the sql-escaping procedure elsewhere, as I recall. In fact, there is an optimisation to the patch that fixed this problem which I chose not to apply, which removes the specific username escaping after this change was made to escape _all_ queries.
So unless someone's rewritten the SQL queries to use User-Password, this is only exploitable by the NAS, whom we trust enough to let them write accounting information to the database anyway. Otherwise, to exploit this bug, you'd need details from two files in /etc/freeradius, which should only be readable by root and the freerad user. (To get the SQL query, and the NAS shared secret) The buffer overflow is _very_ hard to exploit as the buffer the output is being written to is 4096 bytes, and a single RADIUS attribute may only be up to 253 bytes. The longest query in the system is the stop_alt query (used when a stop is received without a start record) at about 1k, which contains 23 radius substitutions, so it may indeed be pushed out to a total length of 6k. However, only an accepted NAS may send data into this query (or in fact any module) so it's only a problem for people who're using FreeRADIUS in a situation where a NAS may try and break into the system. None of the buffers being written to are the first declared in their functions, so unless C is free to reorder stack-local variables (maybe it is?) this can't push into the return address of the stack. So this buffer overflow doesn't render the package unusable, cause data loss or introduce a security hole in the account of the users of this package. (Although that last may depend on what you mean by 'users'. People do not _run_ FreeRADIUS, they query it. It has its own user by default just in case something like this slips through.) And just to remind people, I _upgraded_ this report from wishlist to minor, on the grounds that it is trivial to fix and doesn't affect the usefulness of the package. On further reflection, maybe it should have been 'normal', since it only affects one particular option (rlm_sql) rather than the core server. I certainly don't consider it grave, criticial, or release-critical, but I'm gonna let this sit for a bit before I consider changing the severity, since it's fixed now so it's much less important now. (BTW, I loved the Security Focus comment "No exploit is required.". ^_^) [1] [1] http://www.securityfocus.com/bid/13540/exploit/ -- Paul "TBBle" Hampson, [EMAIL PROTECTED] 7th year CompSci/Asian Studies student, ANU Shorter .sig for a more eco-friendly paperless office.
pgpB9aHxsc9MM.pgp
Description: PGP signature
