security(lite): harden the C-ABI string boundary + backend result parsing
- lite_owned_string: the destructor and move-assignment no longer copy the raw value before freeing (nothing reads the copy there, and a std::string copy can throw bad_alloc — from the noexcept destructor that meant std::terminate, and it could also double-attempt the copy after a throw). They now free only; the copy happens solely in intoResult(), which also moves (not copies) the value into its result so no extra plaintext lingers. - lite_result_parsers: jsonToUnsigned rejected nothing on the string path, so std::stoull accepted "-1" and wrapped it to a huge balance; now requires a leading digit. jsonToSigned rejects leading whitespace/'+'. The outgoing- metadata sum rejects negative values and guards against signed overflow (UB), so a hostile/buggy backend can't forge or overflow a transaction amount. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -40,7 +40,8 @@ LiteBridgeOwnedString::LiteBridgeOwnedString(LiteBridgeOwnedString&& other) noex
|
|||||||
LiteBridgeOwnedString& LiteBridgeOwnedString::operator=(LiteBridgeOwnedString&& other) noexcept
|
LiteBridgeOwnedString& LiteBridgeOwnedString::operator=(LiteBridgeOwnedString&& other) noexcept
|
||||||
{
|
{
|
||||||
if (this == &other) return *this;
|
if (this == &other) return *this;
|
||||||
copyValueBeforeFree();
|
// Free only — do NOT copy the raw value here: nothing reads copiedValue_ after this, and the copy
|
||||||
|
// (std::string assignment) can throw bad_alloc, which from this noexcept path would call std::terminate.
|
||||||
releaseRawValue();
|
releaseRawValue();
|
||||||
wipeString(copiedValue_);
|
wipeString(copiedValue_);
|
||||||
moveFrom(std::move(other));
|
moveFrom(std::move(other));
|
||||||
@@ -49,7 +50,8 @@ LiteBridgeOwnedString& LiteBridgeOwnedString::operator=(LiteBridgeOwnedString&&
|
|||||||
|
|
||||||
LiteBridgeOwnedString::~LiteBridgeOwnedString()
|
LiteBridgeOwnedString::~LiteBridgeOwnedString()
|
||||||
{
|
{
|
||||||
copyValueBeforeFree();
|
// Free only (see operator= note): the copy is unnecessary when the object is dying and must never
|
||||||
|
// throw out of a destructor. The copy happens solely in intoResult(), the one path that reads it.
|
||||||
releaseRawValue();
|
releaseRawValue();
|
||||||
wipeString(copiedValue_);
|
wipeString(copiedValue_);
|
||||||
}
|
}
|
||||||
@@ -66,13 +68,16 @@ LiteBridgeStringResult LiteBridgeOwnedString::intoResult()
|
|||||||
return {false, {}, "lite bridge freeString function is unavailable"};
|
return {false, {}, "lite bridge freeString function is unavailable"};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// copyValueBeforeFree() can throw bad_alloc; if it does, rawValue_ is still set and the destructor
|
||||||
|
// (now free-only) releases it — no leak, no re-copy, no terminate.
|
||||||
copyValueBeforeFree();
|
copyValueBeforeFree();
|
||||||
releaseRawValue();
|
releaseRawValue();
|
||||||
|
|
||||||
const std::string resultValue = copiedValue_;
|
// Move the copy into the result instead of making a second const copy that would linger unwiped.
|
||||||
|
std::string resultValue = std::move(copiedValue_);
|
||||||
wipeString(copiedValue_);
|
wipeString(copiedValue_);
|
||||||
if (looksLikeError(resultValue)) return {false, {}, resultValue};
|
if (looksLikeError(resultValue)) return {false, {}, resultValue};
|
||||||
return {true, resultValue, {}};
|
return {true, std::move(resultValue), {}};
|
||||||
}
|
}
|
||||||
|
|
||||||
void LiteBridgeOwnedString::copyValueBeforeFree()
|
void LiteBridgeOwnedString::copyValueBeforeFree()
|
||||||
|
|||||||
@@ -95,6 +95,9 @@ bool jsonToUnsigned(const json& value, std::uint64_t& output)
|
|||||||
if (value.is_string()) {
|
if (value.is_string()) {
|
||||||
const auto text = value.get<std::string>();
|
const auto text = value.get<std::string>();
|
||||||
if (text.empty()) return false;
|
if (text.empty()) return false;
|
||||||
|
// stoull follows strtoull: it accepts a leading '-' (wrapping to a huge value), '+' and
|
||||||
|
// whitespace. Require a leading digit so a hostile/buggy backend can't forge a balance.
|
||||||
|
if (!std::isdigit(static_cast<unsigned char>(text[0]))) return false;
|
||||||
std::size_t parsedSize = 0;
|
std::size_t parsedSize = 0;
|
||||||
const auto parsed = std::stoull(text, &parsedSize, 10);
|
const auto parsed = std::stoull(text, &parsedSize, 10);
|
||||||
if (parsedSize != text.size()) return false;
|
if (parsedSize != text.size()) return false;
|
||||||
@@ -125,6 +128,8 @@ bool jsonToSigned(const json& value, std::int64_t& output)
|
|||||||
if (value.is_string()) {
|
if (value.is_string()) {
|
||||||
const auto text = value.get<std::string>();
|
const auto text = value.get<std::string>();
|
||||||
if (text.empty()) return false;
|
if (text.empty()) return false;
|
||||||
|
// Reject leading whitespace / '+' that strtoll would otherwise accept (a '-' is valid here).
|
||||||
|
if (text[0] != '-' && !std::isdigit(static_cast<unsigned char>(text[0]))) return false;
|
||||||
std::size_t parsedSize = 0;
|
std::size_t parsedSize = 0;
|
||||||
const auto parsed = std::stoll(text, &parsedSize, 10);
|
const auto parsed = std::stoll(text, &parsedSize, 10);
|
||||||
if (parsedSize != text.size()) return false;
|
if (parsedSize != text.size()) return false;
|
||||||
@@ -337,6 +342,10 @@ bool parseTransactionRecord(const json& value,
|
|||||||
for (std::size_t index = 0; index < metadata.size(); ++index) {
|
for (std::size_t index = 0; index < metadata.size(); ++index) {
|
||||||
LiteTransactionOutput parsedOutput;
|
LiteTransactionOutput parsedOutput;
|
||||||
if (!parseTransactionOutput(metadata[index], path + ".outgoing_metadata[" + std::to_string(index) + "]", parsedOutput, result)) return false;
|
if (!parseTransactionOutput(metadata[index], path + ".outgoing_metadata[" + std::to_string(index) + "]", parsedOutput, result)) return false;
|
||||||
|
// Reject negative values and guard the running sum: signed overflow is UB, and a hostile
|
||||||
|
// backend could otherwise overflow the total into a bogus (even negative) amount.
|
||||||
|
if (parsedOutput.value < 0) return false;
|
||||||
|
if (output.amount > std::numeric_limits<std::int64_t>::max() - parsedOutput.value) return false;
|
||||||
output.amount += parsedOutput.value;
|
output.amount += parsedOutput.value;
|
||||||
output.outgoingMetadata.push_back(std::move(parsedOutput));
|
output.outgoingMetadata.push_back(std::move(parsedOutput));
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user