New activity for FreeRADIUS (the high performance and highly configurable RADIUS server) ====== Replace strncat() with strlcpy() Alan T. DeKok@2015-02-01T23:24:23Z Files modified: * src/main/log.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/0824dd5325f31d2fd2f4e... ====== Merge pull request #895 from spbnick/v2.x.x_misc_fixes v2.x.x misc fixes Alan DeKok@2015-02-01T23:12:02Z Files modified: * src/lib/dhcp.c * src/main/command.c * src/main/conffile.c * src/main/dhcpd.c * src/main/log.c * src/main/modcall.c * src/main/radconf2xml.c * src/main/radmin.c * src/main/threads.c * src/main/xlat.c * src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c * src/modules/rlm_expr/rlm_expr.c * src/modules/rlm_otp/otp_radstate.c * src/modules/rlm_pap/rlm_pap.c * src/modules/rlm_sql/rlm_sql.c * src/modules/rlm_sql_log/rlm_sql_log.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/0495f31aa9255a343e731... ====== Don't dereference NULL cs in cf_item_parse Avoid dereferencing NULL cs in cf_item_parse and cf_reference_item it invokes. This fixes the following Coverity errors: Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:932: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:938: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:958: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:963: var_deref_model: Passing null pointer "cs" to "cf_expand_variables", which dereferences it. freeradius-server-2.2.6/src/main/conffile.c:782:4: deref_parm_in_call: Function "cf_reference_item" dereferences "outercs". freeradius-server-2.2.6/src/main/conffile.c:597:25: var_assign_parm: Assigning: "cs" = "outercs". freeradius-server-2.2.6/src/main/conffile.c:615:4: deref_var: Dereferencing "cs" (which is a copy of "outercs"). Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:958: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:973: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:994: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:1009: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:1041: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:1051: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:1054: var_deref_op: Dereferencing null pointer "cs". Error: FORWARD_NULL (CWE-476): freeradius-server-2.2.6/src/main/conffile.c:900: var_compare_op: Comparing "cs" to null implies that "cs" might be null. freeradius-server-2.2.6/src/main/conffile.c:1066: var_deref_op: Dereferencing null pointer "cs". Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/conffile.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/84a901901ad45ab416514... ====== dhcp: Remove useless variable initializer Remove an initialization of a variable, which is then overwritten, in dhcp_get_option. This fixes the following Clang warning: freeradius-server-2.2.6/src/lib/dhcp.c:144:11: warning: Value stored to 'data' during its initialization is never read Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/lib/dhcp.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/5a2848bd6463476ff7006... ====== dhcpd: Verify DICT_VALUE exists itself Verify that a DICT_VALUE was returned from dict_valbyattr by checking the returned pointer, not the "name" field address. This likely fixes a possible segfault when debugging. This also fixes the following Coverity error: Error: NO_EFFECT (CWE-398): freeradius-server-2.2.6/src/main/dhcpd.c:300: array_null: Comparing an array to null is not useful: "dv->name". Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/dhcpd.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/df1013f5b0d642c20aded... ====== dhcp: Use correct format specifiers in a message Format size_t with %zu specifier, instead of %d, as size_t is not guaranteed to be the same size as int. This fixes the following compiler warnings: freeradius-server-2.2.6/src/lib/dhcp.c: scope_hint: In function 'fr_dhcp_add_arp_entry' freeradius-server-2.2.6/src/lib/dhcp.c:1536: warning: format '%d' expects type 'int', but argument 2 has type 'long unsigned int' freeradius-server-2.2.6/src/lib/dhcp.c:1536: warning: format '%d' expects type 'int', but argument 3 has type 'size_t' Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/lib/dhcp.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/ff526ad4ad985b94dc6fd... ====== Fix two pointer signedness warnings This fixes the following compiler warnings: freeradius-server-2.2.6/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c: scope_hint: In function 'cbtls_verify' freeradius-server-2.2.6/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c:711: warning: pointer targets in passing argument 2 of 'pairmake' differ in signedness freeradius-server-2.2.6/src/include/libradius.h:373: note: expected 'const char *' but argument is of type 'unsigned char *' freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c: scope_hint: In function 'base64_to_hex_xlat' freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:678: warning: pointer targets in passing argument 1 of 'fr_bin2hex' differ in signedness freeradius-server-2.2.6/src/include/libradius.h:418: note: expected 'const uint8_t *' but argument is of type 'char *' Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c * src/modules/rlm_expr/rlm_expr.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/66727f8af40dd5646bb52... ====== Remove two unused variable declarations This fixes the following compiler warnings: freeradius-server-2.2.6/src/main/listen.c:1359: included_from: Included from here. freeradius-server-2.2.6/src/main/command.c: scope_hint: In function 'command_show_module_status' freeradius-server-2.2.6/src/main/command.c:589: warning: unused variable 'mod' freeradius-server-2.2.6/src/modules/rlm_sql/rlm_sql.c: scope_hint: In function 'rlm_sql_detach' freeradius-server-2.2.6/src/modules/rlm_sql/rlm_sql.c:824: warning: unused variable 'i' Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/command.c * src/modules/rlm_sql/rlm_sql.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/fc0f61d714cf21f2d925b... ====== Limit log level string when building message Use strncat instead of strcat to limit the length of copied log level name in radlog_request. This makes the code easier to reason about overall and to make sure no buffer overflow happens. This fixes the following Coverity error: Error: STRING_OVERFLOW (CWE-120): freeradius-server-2.2.6/src/main/log.c:310: fixed_size_dest: You might overrun the 1024 byte fixed-size string "buffer" by copying the return value of "fr_int2str" without checking the length. Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/log.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/a8f4cdea102441523f875... ====== Initialize child count in modcall_recurse Initialize child count in modcall_recurse to explicitly handle the case of no children, making the code at least slightly easier to read, if not actually fixing an issue. This fixes the following Coverity error: Error: UNINIT (CWE-457): freeradius-server-2.2.6/src/main/modcall.c:691: var_decl: Declaring variable "count" without initializer. freeradius-server-2.2.6/src/main/modcall.c:727: uninit_use: Using uninitialized value "count". Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/modcall.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/ba452500fec5456d56cd0... ====== Check cf_item_parse return value Check cf_item_parse return value in dhcp_socket_parse to match all other invocations. This fixes the following Coverity errors: Error: CHECKED_RETURN (CWE-252): freeradius-server-2.2.6/src/main/dhcpd.c:618: check_return: Calling "cf_item_parse" without checking return value (as is done elsewhere 12 out of 14 times). Error: CHECKED_RETURN (CWE-252): freeradius-server-2.2.6/src/main/dhcpd.c:624: check_return: Calling "cf_item_parse" without checking return value (as is done elsewhere 12 out of 14 times). Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/dhcpd.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/30185c3f33d0ffb21b0dc... ====== Check radius_get_vp return value Check radius_get_vp return value in modcall_recurse to match other invocations. This fixes the following Coverity error: Error: CHECKED_RETURN (CWE-252): freeradius-server-2.2.6/src/main/modcall.c:649: check_return: Calling "radius_get_vp" without checking return value (as is done elsewhere 6 out of 7 times). Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/modcall.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/ccf298652f8845d02a13f... ====== Return positive integers from each radius_xlat Make all versions of radius_xlat return positive integers only, including stubs, to match the result checking. This fixes the following Coverity errors: Error: NEGATIVE_RETURNS (CWE-394): freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:625: negative_return_fn: Function "radius_xlat(buffer, 1024, fmt, request, func)" returns a negative number. freeradius-server-2.2.6/src/main/radconf2xml.c:52:2: return_negative_constant: Explicitly returning negative value "-1". freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:625: var_assign: Assigning: unsigned variable "len" = "radius_xlat". freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:637: negative_returns: "len" is passed to a parameter that cannot be negative. freeradius-server-2.2.6/src/lib/base64.c:66:50: sizet: "inlen" is a size_t parameter. Error: NEGATIVE_RETURNS (CWE-394): freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:659: negative_return_fn: Function "radius_xlat(buffer, 1024, fmt, request, func)" returns a negative number. freeradius-server-2.2.6/src/main/radconf2xml.c:52:2: return_negative_constant: Explicitly returning negative value "-1". freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:659: var_assign: Assigning: unsigned variable "len" = "radius_xlat". freeradius-server-2.2.6/src/modules/rlm_expr/rlm_expr.c:666: negative_returns: "len" is passed to a parameter that cannot be negative. freeradius-server-2.2.6/src/lib/base64.c:315:3: parm_loop_bound: Using unsigned parameter "inlen" in a loop exit test. Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/radconf2xml.c * src/main/radmin.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/531d47bf6eccd87849de3... ====== rlm_sql_log: Check rad_mkdir result Check the result of rad_mkdir in sql_log_write, abort on error. This makes the reported error clearer and fixes the following Coverity error: Error: CHECKED_RETURN (CWE-252): freeradius-server-2.2.6/src/modules/rlm_sql_log/rlm_sql_log.c:374: check_return: Calling "rad_mkdir" without checking return value (as is done elsewhere 4 out of 5 times). Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/modules/rlm_sql_log/rlm_sql_log.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/b30632a21044bcffec0f5... ====== xlat: Always free head Move "head" deallocation into the path for handling fr_dhcp_decode_options failure, in xlat_dhcp_options. This makes sure it is freed, as fr_dhcp_decode_options is complicated and is not documented to guarantee deallocation in case of error. This fixes the following Coverity error: Error: RESOURCE_LEAK (CWE-772): freeradius-server-2.2.6/src/main/xlat.c:639: alloc_arg: "fr_dhcp_decode_options" allocates memory that is stored into "head". freeradius-server-2.2.6/src/lib/dhcp.c:581:2: var_assign_parm: Assigning: "tail" = "head". freeradius-server-2.2.6/src/lib/dhcp.c:647:4: alloc_fn: Storage is returned from allocation function "pairmake". freeradius-server-2.2.6/src/lib/valuepair.c:1523:3: alloc_fn: Storage is returned from allocation function "pairmake_any". freeradius-server-2.2.6/src/lib/valuepair.c:1406:2: alloc_fn: Storage is returned from allocation function "paircreate". freeradius-server-2.2.6/src/lib/valuepair.c:174:2: alloc_fn: Storage is returned from allocation function "pairalloc". freeradius-server-2.2.6/src/lib/valuepair.c:72:2: alloc_fn: Storage is returned from allocation function "malloc". freeradius-server-2.2.6/src/lib/valuepair.c:72:2: var_assign: Assigning: "vp" = "malloc(312UL + name_len)". freeradius-server-2.2.6/src/lib/valuepair.c:74:2: noescape: Resource "vp" is not freed or pointed-to in function "memset". [Note: The source code implementation of the function has been overridden by a builtin model.] freeradius-server-2.2.6/src/lib/valuepair.c:134:2: return_alloc: Returning allocated memory "vp". freeradius-server-2.2.6/src/lib/valuepair.c:174:2: var_assign: Assigning: "vp" = "pairalloc(da)". freeradius-server-2.2.6/src/lib/valuepair.c:185:2: return_alloc: Returning allocated memory "vp". freeradius-server-2.2.6/src/lib/valuepair.c:1406:2: var_assign: Assigning: "vp" = "paircreate(attr, 5)". freeradius-server-2.2.6/src/lib/valuepair.c:1466:2: return_alloc: Returning allocated memory "vp". freeradius-server-2.2.6/src/lib/valuepair.c:1523:3: return_alloc_fn: Directly returning storage allocated by "pairmake_any". freeradius-server-2.2.6/src/lib/dhcp.c:647:4: var_assign: Assigning: "vp" = "pairmake(da->name, NULL, T_OP_ADD)". freeradius-server-2.2.6/src/lib/dhcp.c:671:4: var_assign: Assigning: "*tail" = "vp". freeradius-server-2.2.6/src/main/xlat.c:661: leaked_storage: Variable "head" going out of scope leaks the storage it points to. Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/xlat.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/dfdd4e7755e4d21595d5e... ====== rlm_otp: Fix key size calculation Take size of key array element, instead of an expression attempting to calculate the key size, in otp_gen_state(). This makes the HMAC use the full key, instead of just 4 first bytes, increasing key strength. This also fixes the following Coverity error: Error: BAD_SIZEOF (CWE-467): freeradius-server-2.2.6/src/modules/rlm_otp/otp_radstate.c:100: bad_sizeof: Taking the size of arithmetic expression "key[0] * 16" is suspicious. freeradius-server-2.2.6/src/modules/rlm_otp/otp_radstate.c:100: remediation: Did you intend "sizeof (key[0]) * 16"? Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/modules/rlm_otp/otp_radstate.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/59e30f7dfb107f0d5b16b... ====== rlm_pap: Account for terminating zero Account for terminating '\0' character in target (raw) buffer space when verifying supplied vp->length in rlm_pap.c normify(). Otherwise both the source (vp->vp_octets) and the target (raw) buffer will overflow with vp->length == sizeof(raw). This fixes the following Coverity errors: Error: OVERRUN (CWE-119): freeradius-server-2.2.6/src/modules/rlm_pap/rlm_pap.c:260: cond_at_most: Checking "vp->length > 255UL" implies that "vp->length" has the value which may be up to 255 on the false branch. freeradius-server-2.2.6/src/modules/rlm_pap/rlm_pap.c:262: overrun-buffer-arg: Overrunning array "vp->data.octets" of 254 bytes by passing it to a function which accesses it at byte offset 254 using argument "vp->length" (which evaluates to 255). [Note: The source code implementation of the function has been overridden by a builtin model.] Error: OVERRUN (CWE-119): freeradius-server-2.2.6/src/modules/rlm_pap/rlm_pap.c:260: cond_at_most: Checking "vp->length > 255UL" implies that "vp->length" has the value which may be up to 255 on the false branch. freeradius-server-2.2.6/src/modules/rlm_pap/rlm_pap.c:263: overrun-local: Overrunning array "raw" of 255 bytes at byte offset 255 using index "vp->length" (which evaluates to 255). Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/modules/rlm_pap/rlm_pap.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/d2fa6281c0a431aa1e1ea... ====== Verify start_servers <= max_servers Nikolai Kondrashov@2015-01-30T17:16:32Z Files modified: * src/main/threads.c Commit diff: https://github.com/FreeRADIUS/freeradius-server/commit/8c7c238b83c0116dc54f1... ====== -- This commit summary was generated @2015-02-02T00:00:02Z by lgfeed version 0.00 (https://github.com/arr2036/lgfeed).