common: json_to_s64 accept only valid JSON numbers

bool str_to_s64 parses a decimal s64 from exactly buflen bytes; false on bad chars, overflow, leading +, hex like '0x...' . Similar behavior of json_to_u64 and str_to_u64. Tests added.

Author: guibsa

common/json_parse_simple.c

@@ -64,31 +64,9 @@ bool json_to_u64(const char *buffer, const jsmntok_t *tok, u64 *num)
 	return str_to_u64(buffer + tok->start, tok->end - tok->start, num);
 }
 
-/* Uncommon, we don't optimize these */
 bool json_to_s64(const char *buffer, const jsmntok_t *tok, s64 *num)
 {
-	const char *tmpbuf = json_strdup(tmpctx, buffer, tok);
-	char *end;
-	long long l;
-
-	errno = 0;
-	l = strtoll(tmpbuf, &end, 0);
-	if (tmpbuf[0] == '\0' || *end != '\0')
-		return false;
-
-	BUILD_ASSERT(sizeof(l) >= sizeof(*num));
-	*num = l;
-
-	/* Check for overflow/underflow */
-	if ((l == LONG_MAX || l == LONG_MIN) && errno == ERANGE)
-		return false;
-
-	/* Check if the number did not fit in `s64` (in case `long long`
-	is a bigger type). */
-	if (*num != l)
-		return false;
-
-	return true;
+	return str_to_s64(buffer + tok->start, tok->end - tok->start, num);
 }
 
 bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num)

common/test/run-utils-str_to_s64.c

@@ -0,0 +1,234 @@
+#include "config.h"
+#include <assert.h>
+#include <common/amount.h>
+#include <common/pseudorand.h>
+#include <common/setup.h>
+#include <common/utils.h>
+#include <stdio.h>
+#include <wire/wire.h>
+
+/* AUTOGENERATED MOCKS START */
+/* Generated stub for amount_asset_is_main */
+bool amount_asset_is_main(struct amount_asset *asset UNNEEDED)
+{ fprintf(stderr, "amount_asset_is_main called!\n"); abort(); }
+/* Generated stub for amount_asset_to_sat */
+struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED)
+{ fprintf(stderr, "amount_asset_to_sat called!\n"); abort(); }
+/* Generated stub for amount_feerate */
+ bool amount_feerate(u32 *feerate UNNEEDED, struct amount_sat fee UNNEEDED, size_t weight UNNEEDED)
+{ fprintf(stderr, "amount_feerate called!\n"); abort(); }
+/* Generated stub for amount_sat */
+struct amount_sat amount_sat(u64 satoshis UNNEEDED)
+{ fprintf(stderr, "amount_sat called!\n"); abort(); }
+/* Generated stub for amount_sat_add */
+ bool amount_sat_add(struct amount_sat *val UNNEEDED,
+				       struct amount_sat a UNNEEDED,
+				       struct amount_sat b UNNEEDED)
+{ fprintf(stderr, "amount_sat_add called!\n"); abort(); }
+/* Generated stub for amount_sat_eq */
+bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
+{ fprintf(stderr, "amount_sat_eq called!\n"); abort(); }
+/* Generated stub for amount_sat_greater_eq */
+bool amount_sat_greater_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
+{ fprintf(stderr, "amount_sat_greater_eq called!\n"); abort(); }
+/* Generated stub for amount_sat_sub */
+ bool amount_sat_sub(struct amount_sat *val UNNEEDED,
+				       struct amount_sat a UNNEEDED,
+				       struct amount_sat b UNNEEDED)
+{ fprintf(stderr, "amount_sat_sub called!\n"); abort(); }
+/* Generated stub for amount_sat_to_asset */
+struct amount_asset amount_sat_to_asset(struct amount_sat *sat UNNEEDED, const u8 *asset UNNEEDED)
+{ fprintf(stderr, "amount_sat_to_asset called!\n"); abort(); }
+/* Generated stub for amount_tx_fee */
+struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED)
+{ fprintf(stderr, "amount_tx_fee called!\n"); abort(); }
+/* Generated stub for fromwire */
+const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
+{ fprintf(stderr, "fromwire called!\n"); abort(); }
+/* Generated stub for fromwire_bool */
+bool fromwire_bool(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_bool called!\n"); abort(); }
+/* Generated stub for fromwire_fail */
+void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_fail called!\n"); abort(); }
+/* Generated stub for fromwire_secp256k1_ecdsa_signature */
+void fromwire_secp256k1_ecdsa_signature(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
+					secp256k1_ecdsa_signature *signature UNNEEDED)
+{ fprintf(stderr, "fromwire_secp256k1_ecdsa_signature called!\n"); abort(); }
+/* Generated stub for fromwire_sha256 */
+void fromwire_sha256(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct sha256 *sha256 UNNEEDED)
+{ fprintf(stderr, "fromwire_sha256 called!\n"); abort(); }
+/* Generated stub for fromwire_tal_arrn */
+u8 *fromwire_tal_arrn(const tal_t *ctx UNNEEDED,
+		       const u8 **cursor UNNEEDED, size_t *max UNNEEDED, size_t num UNNEEDED)
+{ fprintf(stderr, "fromwire_tal_arrn called!\n"); abort(); }
+/* Generated stub for fromwire_u32 */
+u32 fromwire_u32(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_u32 called!\n"); abort(); }
+/* Generated stub for fromwire_u64 */
+u64 fromwire_u64(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_u64 called!\n"); abort(); }
+/* Generated stub for fromwire_u8 */
+u8 fromwire_u8(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_u8 called!\n"); abort(); }
+/* Generated stub for fromwire_u8_array */
+void fromwire_u8_array(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, u8 *arr UNNEEDED, size_t num UNNEEDED)
+{ fprintf(stderr, "fromwire_u8_array called!\n"); abort(); }
+/* Generated stub for siphash_seed */
+const struct siphash_seed *siphash_seed(void)
+{ fprintf(stderr, "siphash_seed called!\n"); abort(); }
+/* Generated stub for towire */
+void towire(u8 **pptr UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED)
+{ fprintf(stderr, "towire called!\n"); abort(); }
+/* Generated stub for towire_bool */
+void towire_bool(u8 **pptr UNNEEDED, bool v UNNEEDED)
+{ fprintf(stderr, "towire_bool called!\n"); abort(); }
+/* Generated stub for towire_secp256k1_ecdsa_signature */
+void towire_secp256k1_ecdsa_signature(u8 **pptr UNNEEDED,
+			      const secp256k1_ecdsa_signature *signature UNNEEDED)
+{ fprintf(stderr, "towire_secp256k1_ecdsa_signature called!\n"); abort(); }
+/* Generated stub for towire_sha256 */
+void towire_sha256(u8 **pptr UNNEEDED, const struct sha256 *sha256 UNNEEDED)
+{ fprintf(stderr, "towire_sha256 called!\n"); abort(); }
+/* Generated stub for towire_u32 */
+void towire_u32(u8 **pptr UNNEEDED, u32 v UNNEEDED)
+{ fprintf(stderr, "towire_u32 called!\n"); abort(); }
+/* Generated stub for towire_u64 */
+void towire_u64(u8 **pptr UNNEEDED, u64 v UNNEEDED)
+{ fprintf(stderr, "towire_u64 called!\n"); abort(); }
+/* Generated stub for towire_u8 */
+void towire_u8(u8 **pptr UNNEEDED, u8 v UNNEEDED)
+{ fprintf(stderr, "towire_u8 called!\n"); abort(); }
+/* Generated stub for towire_u8_array */
+void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNEEDED)
+{ fprintf(stderr, "towire_u8_array called!\n"); abort(); }
+/* AUTOGENERATED MOCKS END */
+
+#include <assert.h>
+#include <stdint.h>
+
+static void test_str_to_s64(void)
+{
+	s64 val;
+
+	/* Empty input */
+	assert(!str_to_s64("", 0, &val));
+
+	/* Single digit */
+	assert(str_to_s64("0", 1, &val) && val == 0);
+	assert(str_to_s64("5", 1, &val) && val == 5);
+	assert(str_to_s64("9", 1, &val) && val == 9);
+
+	/* Negative single digit */
+	assert(str_to_s64("-0", 2, &val) && val == 0);
+	assert(str_to_s64("-5", 2, &val) && val == -5);
+	assert(str_to_s64("-9", 2, &val) && val == -9);
+
+	/* Multi-digit */
+	assert(str_to_s64("123", 3, &val) && val == 123);
+	assert(str_to_s64("-123", 4, &val) && val == -123);
+	assert(str_to_s64("000", 3, &val) && val == 0);
+	assert(str_to_s64("000123", 6, &val) && val == 123);
+	assert(str_to_s64("-000123", 7, &val) && val == -123);
+
+	/* Exact length prefix */
+	assert(str_to_s64("1234", 2, &val) && val == 12);
+	assert(str_to_s64("-1234", 3, &val) && val == -12);
+
+	/* Max positive value (2^63-1) */
+	assert(str_to_s64("9223372036854775807", 19, &val) &&
+	       val == 9223372036854775807LL);
+
+	/* Min negative value (-2^63) */
+	assert(str_to_s64("-9223372036854775808", 20, &val) &&
+	       val == -9223372036854775807LL - 1);
+
+	/* Overflow (positive) */
+	assert(!str_to_s64("9223372036854775808", 19, &val)); /* 2^63 */
+	assert(!str_to_s64("92233720368547758080", 20, &val));
+
+	/* Overflow (negative) */
+	assert(!str_to_s64("-9223372036854775809", 20, &val)); /* -2^63 - 1 */
+
+	/* Invalid characters */
+	assert(!str_to_s64("12a3", 4, &val));
+	assert(!str_to_s64("1.23", 4, &val));
+	assert(!str_to_s64("123 ", 4, &val));
+	assert(!str_to_s64(" 123", 4, &val));
+	assert(!str_to_s64("--123", 5, &val)); /* Multiple signs */
+	assert(!str_to_s64("+-123", 5, &val)); /* Mixed signs */
+
+	/* Non-digit at various positions */
+	assert(!str_to_s64("x", 1, &val));
+	assert(!str_to_s64("1x", 2, &val));
+	assert(!str_to_s64("12x", 3, &val));
+	assert(!str_to_s64("-x", 2, &val));
+	assert(!str_to_s64("-1x", 3, &val));
+
+	/* Long string of zeros (21) must succeed as 0 */
+	assert(str_to_s64("000000000000000000000", 21, &val) && val == 0);
+	assert(str_to_s64("-000000000000000000000", 22, &val) && val == 0);
+
+	/* Max value with leading zeros */
+	assert(str_to_s64("009223372036854775807", 21, &val) &&
+	       val == 9223372036854775807LL);
+	assert(str_to_s64("-009223372036854775808", 22, &val) &&
+	       val == -9223372036854775807LL - 1);
+
+	/* 1 followed by 19 zeros (10^19), must overflow */
+	assert(!str_to_s64("100000000000000000000", 21, &val));
+
+	/* 1 followed by 18 zeros (10^18), fits */
+	assert(str_to_s64("1000000000000000000", 19, &val) &&
+	       val == 1000000000000000000LL);
+
+	/* 999...9 (19 times) overflows */
+	assert(!str_to_s64("9999999999999999999", 19, &val));
+
+	/* Just below INT64_MAX */
+	assert(str_to_s64("9223372036854775806", 19, &val) &&
+	       val == 9223372036854775806LL);
+
+	/* Just above INT64_MIN */
+	assert(str_to_s64("-9223372036854775807", 20, &val) &&
+	       val == -9223372036854775807LL);
+
+	/* Prefix of the max (first 18 chars of the 19-digit max) */
+	assert(str_to_s64("9223372036854775807", 18, &val) &&
+	       val == 922337203685477580LL);
+
+	/* Prefix of the min (first 19 chars of the 20-digit min) */
+	assert(str_to_s64("-9223372036854775808", 19, &val) &&
+	       val == -922337203685477580LL);
+
+	/* Ensure we do not write *num on failure paths */
+	{
+		s64 before = 0xdeadbeefcafebabeULL;
+		val = before;
+
+		assert(!str_to_s64("12a3", 4, &val));
+		assert(val == before);
+
+		assert(!str_to_s64("", 0, &val));
+		assert(val == before);
+
+		assert(!str_to_s64("9223372036854775808", 19, &val));
+		assert(val == before);
+
+		assert(!str_to_s64("-9223372036854775809", 20, &val));
+		assert(val == before);
+
+		assert(!str_to_s64("1234567890", 0, &val));
+		assert(val == before);
+	}
+}
+
+
+int main(int argc, char *argv[])
+{
+	common_setup(argv[0]);
+	test_str_to_s64();
+	common_shutdown();
+
+	return 0;
+}

common/utils.c

@@ -284,3 +284,41 @@ bool str_to_u64(const char *buf, size_t buflen, u64 *num)
 	*num = val;
 	return true;
 }
+
+bool str_to_s64(const char *buf, size_t buflen, s64 *num)
+{
+	s64 val = 0;
+	bool negative = false;
+	size_t start = 0;
+
+	if (buflen == 0)
+		return false;
+
+	if (buf[0] == '-') {
+		negative = true;
+		start = 1;
+	}
+
+	if (start == buflen)
+		return false;
+
+	for (size_t i = start; i < buflen; i++) {
+		s64 digit;
+		if (buf[i] < '0' || buf[i] > '9')
+			return false;
+		digit = buf[i] - '0';
+
+		if (negative) {
+			if (val < (INT64_MIN + digit) / 10)
+				return false;
+			val = val * 10 - digit;
+		} else {
+			if (val > (INT64_MAX - digit) / 10)
+				return false;
+			val = val * 10 + digit;
+		}
+	}
+
+	*num = val;
+	return true;
+}

common/utils.h

@@ -133,6 +133,9 @@ void tal_arr_appendn_(void *p, const void *append TAKES, size_t bytes);
 /* Parse a decimal u64 from exactly buflen bytes; false on bad chars or overflow */
 bool str_to_u64(const char *buf, size_t buflen, u64 *num);
 
+/* Parse a decimal s64 from exactly buflen bytes; false on bad chars, overflow, or leading + */
+bool str_to_s64(const char *buf, size_t buflen, s64 *num);
+
 /* Check for valid UTF-8 */
 bool utf8_check(const void *buf, size_t buflen);