commit 2fdf4d18f2227880385cff40fd107e7b467a4271
parent b8500e9ef92c9e65fbeb94cd04239f20465c9217
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 17:47:33 +0200
handle theoretical integer overflow in reserve open period
Diffstat:
2 files changed, 197 insertions(+), 10 deletions(-)
diff --git a/src/exchangedb/do_reserve_open.sql b/src/exchangedb/do_reserve_open.sql
@@ -36,10 +36,12 @@ DECLARE
my_balance taler_amount;
my_cost taler_amount;
my_cost_tmp INT8;
- my_years_tmp INT4;
- my_years INT4;
+ my_years_tmp INT8;
+ my_years INT8;
+ my_purses_allowed INT8;
my_needs_update BOOL;
my_expiration_date INT8;
+ my_desired_expiration INT8;
reserve RECORD;
BEGIN
@@ -79,10 +81,20 @@ my_cost.frac = 0;
my_needs_update = FALSE;
my_years = 0;
+-- Bound the requested expiration. GNUNET_JSON_spec_timestamp() accepts the
+-- literal "never" and hands us INT64_MAX, and any t_s above roughly 9.2e12
+-- seconds is clamped to the same value; the additions below would then
+-- overflow INT8 (SQLSTATE 22003), which the caller can only turn into an
+-- HTTP 500. Pricing such a request as if it had asked for the cap gives the
+-- client the ordinary "here is what it would cost" answer -- which it cannot
+-- pay, so it gets a 402 -- instead of an internal error. The cap is the end
+-- of year 9999, the largest timestamp PostgreSQL itself can represent.
+my_desired_expiration = LEAST (in_desired_expiration, 253402300799000000);
+
-- Compute years based on desired expiration time
-IF (my_expiration_date < in_desired_expiration)
+IF (my_expiration_date < my_desired_expiration)
THEN
- my_years = (31535999999999 + in_desired_expiration - my_expiration_date) / 31536000000000;
+ my_years = (31535999999999 + my_desired_expiration - my_expiration_date) / 31536000000000;
reserve.purses_allowed = in_default_purse_limit;
my_expiration_date = my_expiration_date + 31536000000000 * my_years;
END IF;
@@ -90,23 +102,49 @@ END IF;
-- Increase years based on purses requested
IF (reserve.purses_allowed < in_min_purse_limit)
THEN
- my_years = (31535999999999 + in_desired_expiration - in_now) / 31536000000000;
+ my_years = (31535999999999 + my_desired_expiration - in_now) / 31536000000000;
my_expiration_date = in_now + 31536000000000 * my_years;
- my_years_tmp = (in_min_purse_limit + in_default_purse_limit - reserve.purses_allowed - 1) / in_default_purse_limit;
+ -- in_min_purse_limit is an INT4 that comes straight from the client's
+ -- "purse_limit", so this sum has to be evaluated in INT8: in INT4 it
+ -- overflows before the division.
+ my_years_tmp = (in_min_purse_limit::INT8 + in_default_purse_limit - reserve.purses_allowed - 1) / in_default_purse_limit;
my_years = my_years + my_years_tmp;
- reserve.purses_allowed = reserve.purses_allowed + (in_default_purse_limit * my_years_tmp);
+ my_purses_allowed = reserve.purses_allowed::INT8 + (in_default_purse_limit::INT8 * my_years_tmp);
+ELSE
+ my_purses_allowed = reserve.purses_allowed;
END IF;
+-- Refuse to price what we cannot represent, rather than overflowing further
+-- down: reporting the maximum cost makes the request fail with "payment
+-- required", where an arithmetic overflow can only become an HTTP 500.
+IF ( (my_years > 8000) OR
+ (my_purses_allowed > 2147483647) )
+THEN
+ -- Just below TALER_AMOUNT_MAX_VALUE, the largest amount the C layer parses.
+ out_open_cost.val=4503599627370495;
+ out_open_cost.frac=99999999;
+ out_final_expiration=my_expiration_date;
+ out_no_funds=FALSE;
+ RAISE NOTICE 'requested reserve lifetime or purse limit out of range';
+ RETURN;
+END IF;
-- Compute cost based on annual fees
IF (my_years > 0)
THEN
+ -- my_years and taler_amount.frac are multiplied in INT8: with my_years
+ -- declared INT4 this used to be an INT4 multiplication that overflowed
+ -- before the division, making the guard below unreachable.
my_cost.val = my_years * in_open_fee.val;
my_cost_tmp = my_years * in_open_fee.frac / 100000000;
IF (CAST (my_cost.val + my_cost_tmp AS INT8) < my_cost.val)
THEN
- out_open_cost.val=9223372036854775807;
- out_open_cost.frac=2147483647;
+ -- Must stay below TALER_AMOUNT_MAX_VALUE (and 100000000): the previous
+ -- sentinel (INT64_MAX, 2^31-1) is not a legal taler amount, so the C
+ -- layer refused to parse it and turned this branch into an HTTP 500 of
+ -- its own.
+ out_open_cost.val=4503599627370495;
+ out_open_cost.frac=99999999;
out_final_expiration=my_expiration_date;
out_no_funds=FALSE;
RAISE NOTICE 'arithmetic issue computing amount';
@@ -182,7 +220,7 @@ UPDATE reserves SET
current_balance=my_balance
,gc_date=reserve.expiration_date + in_reserve_gc_delay
,expiration_date=my_expiration_date
- ,purses_allowed=reserve.purses_allowed
+ ,purses_allowed=my_purses_allowed
WHERE
reserve_pub=in_reserve_pub;
diff --git a/src/exchangedb/test_regressions.c b/src/exchangedb/test_regressions.c
@@ -31,6 +31,7 @@
#include "exchange-database/start.h"
#include "exchange-database/commit.h"
#include "exchange-database/rollback.h"
+#include "exchange-database/do_reserve_open.h"
/**
@@ -349,6 +350,150 @@ check_commit_detects_rolled_back_transaction (void)
/**
+ * Helper for the reserve-open checks: create a reserve with the given
+ * balance and run exchange_do_reserve_open() on it.
+ *
+ * @param reserve_pub reserve to create and open
+ * @param desired_expiration expiration the client asks for
+ * @param now current time to assume
+ * @param min_purse_limit number of purses the client asks for
+ * @param open_fee annual account fee of the exchange
+ * @param[out] open_cost set to the cost the exchange computed
+ * @return transaction status
+ */
+static enum GNUNET_DB_QueryStatus
+try_reserve_open (const struct TALER_ReservePublicKeyP *reserve_pub,
+ struct GNUNET_TIME_Timestamp desired_expiration,
+ struct GNUNET_TIME_Timestamp now,
+ uint32_t min_purse_limit,
+ const struct TALER_Amount *open_fee,
+ struct TALER_Amount *open_cost)
+{
+ struct TALER_ReserveSignatureP reserve_sig;
+ struct TALER_Amount zero;
+ struct TALER_Amount balance;
+ struct GNUNET_TIME_Timestamp final_expiration;
+ bool no_funds;
+ char *sql;
+
+ memset (&reserve_sig,
+ 0x51,
+ sizeof (reserve_sig));
+ parse_amount ("0",
+ &zero);
+ sql = hex_insert_reserve (reserve_pub);
+ GNUNET_assert (GNUNET_OK ==
+ exec_sql (sql));
+ GNUNET_free (sql);
+ return TALER_EXCHANGEDB_do_reserve_open (pg,
+ reserve_pub,
+ &zero,
+ &zero,
+ min_purse_limit,
+ &reserve_sig,
+ desired_expiration,
+ now,
+ open_fee,
+ &no_funds,
+ &balance,
+ open_cost,
+ &final_expiration);
+}
+
+
+/**
+ * E-3: `{"reserve_expiration":"never"}` is a perfectly well-formed request:
+ * GNUNET_JSON_spec_timestamp() turns it into GNUNET_TIME_UNIT_FOREVER_ABS and
+ * qconv_abs_time() clamps that to INT64_MAX. The stored procedure then
+ * overflowed INT8 computing the number of years, which SQLSTATE 22003 turns
+ * into a hard error and the handler into an HTTP 500.
+ */
+static int
+check_reserve_open_never_expires (void)
+{
+ struct TALER_ReservePublicKeyP reserve_pub;
+ struct TALER_Amount open_fee;
+ struct TALER_Amount open_cost;
+
+ memset (&reserve_pub,
+ 0x33,
+ sizeof (reserve_pub));
+ /* No fractional part here; that is E-4's business. */
+ parse_amount ("1",
+ &open_fee);
+ /* Before the fix: 'bigint out of range' -> HARD_ERROR. */
+ FAILIF (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
+ try_reserve_open (&reserve_pub,
+ GNUNET_TIME_UNIT_FOREVER_TS,
+ GNUNET_TIME_timestamp_get (),
+ 1,
+ &open_fee,
+ &open_cost));
+ return 0;
+}
+
+
+/**
+ * E-4: `my_years * in_open_fee.frac` was an INT4 multiplication that
+ * overflowed *before* the division, so the author's own overflow guard could
+ * never be reached: an exchange whose ACCOUNT_FEE has a fractional part
+ * answered a far-future reserve-open request with an HTTP 500.
+ *
+ * The same block computed the new purse limit in INT4 from the client's
+ * `purse_limit`, which overflows for any value near INT32_MAX.
+ */
+static int
+check_reserve_open_int4_overflows (void)
+{
+ struct TALER_ReservePublicKeyP reserve_pub;
+ struct TALER_Amount open_fee;
+ struct TALER_Amount open_cost;
+ struct TALER_Amount expected;
+ struct GNUNET_TIME_Timestamp now;
+
+ parse_amount ("0.5",
+ &open_fee);
+ parse_amount ("25",
+ &expected);
+
+ /* 50 years at EUR:0.50/year: 50 * 50000000 does not fit into an INT4. */
+ memset (&reserve_pub,
+ 0x34,
+ sizeof (reserve_pub));
+ now = GNUNET_TIME_timestamp_get ();
+ FAILIF (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
+ try_reserve_open (&reserve_pub,
+ GNUNET_TIME_absolute_to_timestamp (
+ GNUNET_TIME_absolute_add (
+ now.abs_time,
+ GNUNET_TIME_relative_multiply (
+ GNUNET_TIME_UNIT_YEARS,
+ 50))),
+ now,
+ 1,
+ &open_fee,
+ &open_cost));
+ FAILIF (0 !=
+ TALER_amount_cmp (&expected,
+ &open_cost));
+
+ /* An absurd purse_limit must be priced out of range, not overflow. */
+ memset (&reserve_pub,
+ 0x35,
+ sizeof (reserve_pub));
+ now = GNUNET_TIME_timestamp_get ();
+ FAILIF (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
+ try_reserve_open (&reserve_pub,
+ now,
+ now,
+ 2147483647,
+ &open_fee,
+ &open_cost));
+ return 0;
+}
+
+
+/**
* All checks we know about.
*/
static const struct
@@ -360,6 +505,10 @@ static const struct
&check_aggregate_refund_below_deposit_fee },
{ "commit-detects-rolled-back-transaction",
&check_commit_detects_rolled_back_transaction },
+ { "reserve-open-never-expires",
+ &check_reserve_open_never_expires },
+ { "reserve-open-int4-overflows",
+ &check_reserve_open_int4_overflows },
{ NULL, NULL }
};
