Skip to content

Commit

Permalink
[Tizen] Improve app-preferences get/set/remove functions (#18792)
Browse files Browse the repository at this point in the history 
* Improve app-preferences get/set/remove functions

Improvements:

- Do not check for key existence before calling get/set functions. The
	key existence status will be reported by these functions anyway.

- Prevent memory leaks with scope allocators instead of manually calling
	free() functions.

- Prevent out-of-bound read when logging non-string values by passing
	the data length to the printf-like function with "%.*s" specifier.

* Do not use ternary "if" for logger format string

Using ternary "if"s with logging macros forbids using simple
augmentation with file name and/or line number, e.g.:

	#define ChipLogError(MOD, MSG, ...) \
	  ::Log(MOD, "%s:%d " MSG, __FILE__, __LINE__, ##__VA_ARGS__)
  • Loading branch information
@arkq @pull
arkq authored and pull[bot] committed Dec 21, 2023
1 parent 0a21503 commit 3429257
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 101 deletions.
157 changes: 59 additions & 98 deletions src/platform/Tizen/AppPreference.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,145 +19,106 @@
#include <app_preference.h>
#include <lib/support/Base64.h>
#include <lib/support/CHIPMem.h>
#include <memory>

namespace chip {
namespace DeviceLayer {
namespace PersistedStorage {
namespace Internal {
namespace AppPreference {

static CHIP_ERROR __IsKeyExist(const char * key)
{
CHIP_ERROR err = CHIP_NO_ERROR;
int preErr = PREFERENCE_ERROR_NONE;
bool isExist = false;

preErr = preference_is_existing(key, &isExist);
if (preErr != PREFERENCE_ERROR_NONE)
{
err = CHIP_ERROR_INCORRECT_STATE;
}
else if (isExist == false)
{
err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

return err;
}

CHIP_ERROR CheckData(const char * key)
{
return __IsKeyExist(key);
bool isExist = false;
int err = preference_is_existing(key, &isExist);
VerifyOrReturnError(err == PREFERENCE_ERROR_NONE, CHIP_ERROR_INCORRECT_STATE);
return isExist ? CHIP_NO_ERROR : CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

CHIP_ERROR GetData(const char * key, void * data, size_t dataSize, size_t * getDataSize, size_t offset)
{
CHIP_ERROR err = CHIP_NO_ERROR;
int preErr = PREFERENCE_ERROR_NONE;
char * encodedData = NULL;
uint8_t * decodedData = NULL;
size_t encodedDataSize = 0;
size_t encodedDataPaddingSize = 0;
size_t decodedDataSize = 0;
size_t expectedDecodedSize = 0;
size_t copy_size = 0;

VerifyOrExit(data != NULL, err = CHIP_ERROR_INVALID_ARGUMENT);

err = __IsKeyExist(key);
VerifyOrExit(err == CHIP_NO_ERROR, ChipLogProgress(DeviceLayer, "Not found data [%s]", key));

preErr = preference_get_string(key, &encodedData);
VerifyOrExit(preErr == PREFERENCE_ERROR_NONE, err = CHIP_ERROR_INCORRECT_STATE);
encodedDataSize = strlen(encodedData);

if ((encodedDataSize > 0) && (encodedData[encodedDataSize - 1] == '='))
{
encodedDataPaddingSize++;
if ((encodedDataSize > 1) && (encodedData[encodedDataSize - 2] == '='))
encodedDataPaddingSize++;
}
expectedDecodedSize = ((encodedDataSize - encodedDataPaddingSize) * 3) / 4;

decodedData = static_cast<uint8_t *>(chip::Platform::MemoryAlloc(expectedDecodedSize));
VerifyOrExit(decodedData != NULL, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(data != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

decodedDataSize = Base64Decode(encodedData, static_cast<uint16_t>(encodedDataSize), decodedData);
char * encodedData = nullptr;
// Make sure that string allocated by preference_get_string() will be freed
std::unique_ptr<char, decltype(&::free)> _{ encodedData, &::free };

copy_size = min(dataSize, decodedDataSize - offset);
if (getDataSize != NULL)
int err = preference_get_string(key, &encodedData);
if (err == PREFERENCE_ERROR_NO_KEY)
{
*getDataSize = copy_size;
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}
if (err == PREFERENCE_ERROR_OUT_OF_MEMORY)
{
return CHIP_ERROR_NO_MEMORY;
}
if (err != PREFERENCE_ERROR_NONE)
{
ChipLogError(DeviceLayer, "Failed to get preference [%s]: %s", key, get_error_message(err));
return CHIP_ERROR_INCORRECT_STATE;
}
memset(data, 0, dataSize);
memcpy(data, decodedData + offset, copy_size);

ChipLogProgress(DeviceLayer, "Get data [%s:%s]", key, (char *) data);
size_t encodedDataSize = strlen(encodedData);

chip::Platform::MemoryFree(decodedData);
Platform::ScopedMemoryBuffer<uint8_t> decodedData;
size_t expectedMaxDecodedSize = BASE64_MAX_DECODED_LEN(encodedDataSize);
VerifyOrReturnError(decodedData.Alloc(expectedMaxDecodedSize), CHIP_ERROR_NO_MEMORY);

VerifyOrExit(dataSize >= decodedDataSize - offset, err = CHIP_ERROR_BUFFER_TOO_SMALL);
size_t decodedDataSize = Base64Decode(encodedData, static_cast<uint16_t>(encodedDataSize), decodedData.Get());
VerifyOrReturnError(dataSize >= decodedDataSize - offset, CHIP_ERROR_BUFFER_TOO_SMALL);

exit:
free(encodedData);
return err;
size_t copySize = std::min(dataSize, decodedDataSize - offset);
if (getDataSize != nullptr)
{
*getDataSize = copySize;
}
::memcpy(data, decodedData.Get() + offset, copySize);

ChipLogProgress(DeviceLayer, "Get data [%s:%.*s]", key, static_cast<int>(copySize), static_cast<char *>(data));
return CHIP_NO_ERROR;
}

CHIP_ERROR SaveData(const char * key, const uint8_t * data, size_t dataSize)
CHIP_ERROR SaveData(const char * key, const void * data, size_t dataSize)
{
CHIP_ERROR err = CHIP_NO_ERROR;
int preErr = PREFERENCE_ERROR_NONE;
char * encodedData = NULL;
size_t encodedDataSize = 0;
size_t expectedEncodedSize = ((dataSize + 3) * 4) / 3;

err = __IsKeyExist(key);
if (err == CHIP_NO_ERROR)
{
VerifyOrExit(RemoveData(key) == CHIP_NO_ERROR, err = CHIP_ERROR_INCORRECT_STATE);
}
err = CHIP_NO_ERROR;
// Expected size for null-terminated base64 string
size_t expectedEncodedSize = BASE64_ENCODED_LEN(dataSize) + 1;

encodedData = static_cast<char *>(chip::Platform::MemoryAlloc(expectedEncodedSize));
VerifyOrExit(encodedData != NULL, err = CHIP_ERROR_INCORRECT_STATE);
Platform::ScopedMemoryBuffer<char> encodedData;
VerifyOrReturnError(encodedData.Alloc(expectedEncodedSize), CHIP_ERROR_NO_MEMORY);

encodedDataSize = Base64Encode(data, static_cast<uint16_t>(dataSize), encodedData);
encodedData[encodedDataSize] = 0;
size_t encodedDataSize = Base64Encode(static_cast<const uint8_t *>(data), static_cast<uint16_t>(dataSize), encodedData.Get());
encodedData[encodedDataSize] = '\0';

preErr = preference_set_string(key, encodedData);
if (preErr == PREFERENCE_ERROR_NONE)
int err = preference_set_string(key, encodedData.Get());
if (err == PREFERENCE_ERROR_OUT_OF_MEMORY)
{
ChipLogProgress(DeviceLayer, "Save data [%s:%s]", key, data);
return CHIP_ERROR_NO_MEMORY;
}
else
if (err != PREFERENCE_ERROR_NONE)
{
ChipLogProgress(DeviceLayer, "FAIL: set string [%s]", get_error_message(preErr));
err = CHIP_ERROR_INCORRECT_STATE;
ChipLogError(DeviceLayer, "Failed to set preference [%s]: %s", key, get_error_message(err));
return CHIP_ERROR_INCORRECT_STATE;
}

chip::Platform::MemoryFree(encodedData);

exit:
return err;
ChipLogProgress(DeviceLayer, "Save data [%s:%.*s]", key, static_cast<int>(dataSize), static_cast<const char *>(data));
return CHIP_NO_ERROR;
}

CHIP_ERROR RemoveData(const char * key)
{
CHIP_ERROR err = CHIP_NO_ERROR;
int preErr = PREFERENCE_ERROR_NONE;

preErr = preference_remove(key);
if (preErr == PREFERENCE_ERROR_NONE)
int err = preference_remove(key);
if (err == PREFERENCE_ERROR_NO_KEY)
{
ChipLogProgress(DeviceLayer, "Remove data [%s]", key);
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}
else
if (err != PREFERENCE_ERROR_NONE)
{
ChipLogProgress(DeviceLayer, "FAIL: remove preference [%s]", get_error_message(preErr));
err = CHIP_ERROR_INCORRECT_STATE;
ChipLogError(DeviceLayer, "Failed to remove preference [%s]: %s", key, get_error_message(err));
return CHIP_ERROR_INCORRECT_STATE;
}

return err;
ChipLogProgress(DeviceLayer, "Remove data [%s]", key);
return CHIP_NO_ERROR;
}

} // namespace AppPreference
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Tizen/AppPreference.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace AppPreference {

CHIP_ERROR CheckData(const char * key);
CHIP_ERROR GetData(const char * key, void * data, size_t dataSize, size_t * getDataSize, size_t offset);
CHIP_ERROR SaveData(const char * key, const uint8_t * data, size_t size);
CHIP_ERROR SaveData(const char * key, const void * data, size_t size);
CHIP_ERROR RemoveData(const char * key);

} // namespace AppPreference
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Tizen/BLEManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void BLEManagerImpl::GattConnectionStateChangedCb(int result, bool connected, co

if (result != BT_ERROR_NONE)
{
ChipLogError(DeviceLayer, connected ? "Connection req failed" : "Disconnection req failed");
ChipLogError(DeviceLayer, "%s", connected ? "Connection req failed" : "Disconnection req failed");
if (connected)
sInstance.NotifyHandleConnectFailed(CHIP_ERROR_INTERNAL);
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Tizen/KeyValueStoreManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t

CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size)
{
return Internal::AppPreference::SaveData(key, reinterpret_cast<const uint8_t *>(value), value_size);
return Internal::AppPreference::SaveData(key, value, value_size);
}

CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key)
Expand Down

0 comments on commit 3429257

Please sign in to comment.