Skip to content

Commit

Permalink
Fix code in CHIPCryptoPALOpenSSL that could try to log nullptr (#18105)
Browse files Browse the repository at this point in the history
* Fix code in CHIPCryptoPALOpenSSL could try to log null

- Some %s specifiers did not have a guaranteed non-null string,
  in OpenSSL error reporting.
- Overall, we are missing a macro/function to make it easy to
  return a default string value when a string is null for a
  `%s` specifier, which is Undefined Behavior. Relying on
  standard libraries "catching it for us" is wrong and may
  fail on some platforms.

This PR makes sure CHIPCryptoPALOpenSSL error reporting never
uses a null string argument. This is now valid by construction
without needing to validate the underlying implementations.

This PR also adds `DefaultStringWhenNull` and `StringOrNullMarker`
utilities to assist others in doing this safely

Testing done:
- Unit tests and cert tests all pass

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Aug 4, 2023
1 parent 470cc4b commit 1339172
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/crypto/CHIPCryptoPALOpenSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ static void _logSSLError()
const char * err_str_reason = ERR_reason_error_string(ssl_err_code);
if (err_str_lib)
{
ChipLogError(Crypto, " ssl err %s %s %s\n", err_str_lib, err_str_routine, err_str_reason);
ChipLogError(Crypto, " ssl err %s %s %s\n", StringOrNullMarker(err_str_lib), StringOrNullMarker(err_str_routine),
StringOrNullMarker(err_str_reason));
}
#endif // CHIP_ERROR_LOGGING
ssl_err_code = ERR_get_error();
Expand Down
23 changes: 23 additions & 0 deletions src/lib/support/CodeUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,29 @@ inline void chipDie(void)
*/
#define ArraySize(a) (sizeof(a) / sizeof((a)[0]))

/**
* @brief Ensures that if `str` is NULL, a non-null `default_str_value` is provided
*
* @param str - null-terminated string pointer or nullptr
* @param default_str_value - replacement value if `str` is nullptr
* @return `str` if not null, otherwise `default_str_value`
*/
inline const char * DefaultStringWhenNull(const char * str, const char * default_str_value)
{
return (str != nullptr) ? str : default_str_value;
}

/**
* @brief Ensure that a string for a %s specifier is shown as "(null)" if null
*
* @param str - null-terminated string pointer or nullptr
* @return `str` if not null, otherwise literal "(null)"
*/
inline const char * StringOrNullMarker(const char * str)
{
return DefaultStringWhenNull(str, "(null)");
}

namespace chip {

/**
Expand Down

0 comments on commit 1339172

Please sign in to comment.