From 23bf1eee74bed73cf98264c247ab44df8dadfcd9 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Wed, 18 Mar 2015 18:55:41 +0100 Subject: [PATCH] fix #3716: make sure amount-API offers proper checks against overflow and other issues --- src/include/taler_amount_lib.h | 121 ++++-- src/include/taler_json_lib.h | 2 +- src/lib/mint_api.c | 12 +- src/mint/mint_db.c | 25 +- src/mint/mint_db.h | 2 +- src/mint/taler-mint-httpd_db.c | 143 +++++--- src/mint/taler-mint-httpd_deposit.c | 3 +- src/mint/taler-mint-httpd_keystate.c | 21 +- src/mint/taler-mint-httpd_parsing.c | 14 +- src/mint/taler-mint-httpd_refresh.c | 40 +- src/mint/taler-mint-httpd_responses.c | 68 +++- src/mint/taler-mint-keyup.c | 27 +- src/mint/taler-mint-reservemod.c | 8 +- src/util/amount.c | 508 ++++++++++++++++---------- src/util/json.c | 21 +- 15 files changed, 668 insertions(+), 347 deletions(-) diff --git a/src/include/taler_amount_lib.h b/src/include/taler_amount_lib.h index 8bea0256c..e64ff4d92 100644 --- a/src/include/taler_amount_lib.h +++ b/src/include/taler_amount_lib.h @@ -25,10 +25,27 @@ /** * Number of characters (plus 1 for 0-termination) we use to * represent currency names (i.e. EUR, USD, etc.). We use - * 8 for alignment (!). + * 4 for alignment as 3 characters are typical and we need a + * 0-terminator. So do not change this. */ #define TALER_CURRENCY_LEN 4 +/** + * The "fraction" value in a `struct TALER_Amount` represents which + * fraction of the "main" value? + * + * Note that we need sub-cent precision here as transaction fees might + * be that low, and as we want to support microdonations. + */ +#define TALER_AMOUNT_FRAC_BASE 1000000 + +/** + * How many digits behind the comma are required to represent the + * fractional value in human readable decimal format? Must match + * lg(#TALER_AMOUNT_FRAC_BASE). + */ +#define TALER_AMOUNT_FRAC_LEN 6 + GNUNET_NETWORK_STRUCT_BEGIN @@ -41,12 +58,12 @@ struct TALER_AmountNBO /** * Value in the main currency, in NBO. */ - uint32_t value; + uint64_t value GNUNET_PACKED; /** * Additinal fractional value, in NBO. */ - uint32_t fraction; + uint32_t fraction GNUNET_PACKED; /** * Type of the currency being represented. @@ -65,7 +82,7 @@ struct TALER_Amount /** * Value (numerator of fraction) */ - uint32_t value; + uint64_t value; /** * Fraction (denominator of fraction) @@ -73,7 +90,8 @@ struct TALER_Amount uint32_t fraction; /** - * Currency string, left adjusted and padded with zeros. + * Currency string, left adjusted and padded with zeros. All zeros + * for "invalid" values. */ char currency[TALER_CURRENCY_LEN]; }; @@ -92,82 +110,123 @@ TALER_string_to_amount (const char *str, struct TALER_Amount *denom); +/** + * Get the value of "zero" in a particular currency. + * + * @param cur currency description + * @param denom denomination to write the result to + * @return #GNUNET_OK if @a cur is a valid currency specification, + * #GNUNET_SYSERR if it is invalid. + */ +int +TALER_amount_get_zero (const char *cur, + struct TALER_Amount *denom); + + /** * Convert amount from host to network representation. * + * @param res where to store amount in network representation * @param d amount in host representation - * @return amount in network representation */ -struct TALER_AmountNBO -TALER_amount_hton (const struct TALER_Amount d); +void +TALER_amount_hton (struct TALER_AmountNBO *res, + const struct TALER_Amount *d); /** * Convert amount from network to host representation. * + * @param res where to store amount in host representation * @param d amount in network representation - * @return amount in host representation */ -struct TALER_Amount -TALER_amount_ntoh (const struct TALER_AmountNBO dn); +void +TALER_amount_ntoh (struct TALER_Amount *res, + const struct TALER_AmountNBO *dn); /** - * Compare the value/fraction of two amounts. Does not compare the currency, - * i.e. comparing amounts with the same value and fraction but different - * currency would return 0. + * Compare the value/fraction of two amounts. Does not compare the currency. + * Comparing amounts of different currencies will cause the program to abort(). + * If unsure, check with #TALER_amount_cmp_currency() first to be sure that + * the currencies of the two amounts are identical. * * @param a1 first amount * @param a2 second amount * @return result of the comparison */ int -TALER_amount_cmp (struct TALER_Amount a1, - struct TALER_Amount a2); +TALER_amount_cmp (const struct TALER_Amount *a1, + const struct TALER_Amount *a2); + + +/** + * Test if @a a1 and @a a2 are the same currency. + * + * @param a1 amount to test + * @param a2 amount to test + * @return #GNUNET_YES if @a a1 and @a a2 are the same currency + * #GNUNET_NO if the currencies are different + * #GNUNET_SYSERR if either amount is invalid + */ +int +TALER_amount_cmp_currency (const struct TALER_Amount *a1, + const struct TALER_Amount *a2); /** * Perform saturating subtraction of amounts. * + * @param diff where to store (@a a1 - @a a2), or invalid if @a a2 > @a a1 * @param a1 amount to subtract from * @param a2 amount to subtract - * @return (a1-a2) or 0 if a2>=a1 + * @return #GNUNET_OK if the subtraction worked, + * #GNUNET_NO if @a a1 = @a a2 + * #GNUNET_SYSERR if @a a2 > @a a1 or currencies are incompatible; + * @a diff is set to invalid */ -struct TALER_Amount -TALER_amount_subtract (struct TALER_Amount a1, - struct TALER_Amount a2); +int +TALER_amount_subtract (struct TALER_Amount *diff, + const struct TALER_Amount *a1, + const struct TALER_Amount *a2); /** - * Perform saturating addition of amounts + * Perform addition of amounts. * + * @param sum where to store @a a1 + @a a2, set to "invalid" on overflow * @param a1 first amount to add * @param a2 second amount to add - * @return sum of a1 and a2 + * @return #GNUNET_OK if the addition worked, + * #GNUNET_SYSERR on overflow */ -struct TALER_Amount -TALER_amount_add (struct TALER_Amount a1, - struct TALER_Amount a2); +int +TALER_amount_add (struct TALER_Amount *sum, + const struct TALER_Amount *a1, + const struct TALER_Amount *a2); /** * Normalize the given amount. * - * @param amout amount to normalize - * @return normalized amount + * @param amount amount to normalize + * @return #GNUNET_OK if normalization worked + * #GNUNET_NO if value was already normalized + * #GNUNET_SYSERR if value was invalid or could not be normalized */ -struct TALER_Amount -TALER_amount_normalize (struct TALER_Amount amount); +int +TALER_amount_normalize (struct TALER_Amount *amount); /** * Convert amount to string. * * @param amount amount to convert to string - * @return freshly allocated string representation + * @return freshly allocated string representation, + * NULL if the @a amount was invalid */ char * -TALER_amount_to_string (struct TALER_Amount amount); +TALER_amount_to_string (const struct TALER_Amount *amount); #endif diff --git a/src/include/taler_json_lib.h b/src/include/taler_json_lib.h index 1048b89f6..c5515966f 100644 --- a/src/include/taler_json_lib.h +++ b/src/include/taler_json_lib.h @@ -39,7 +39,7 @@ * @return a json object describing the amount */ json_t * -TALER_JSON_from_amount (struct TALER_Amount amount); +TALER_JSON_from_amount (const struct TALER_Amount *amount); /** diff --git a/src/lib/mint_api.c b/src/lib/mint_api.c index af4f04956..eb2697a2e 100644 --- a/src/lib/mint_api.c +++ b/src/lib/mint_api.c @@ -412,10 +412,14 @@ parse_json_denomkey (struct TALER_MINT_DenomPublicKey **_denom_key, denom_key_issue.start = GNUNET_TIME_absolute_hton (valid_from); denom_key_issue.expire_withdraw = GNUNET_TIME_absolute_hton (withdraw_valid_until); denom_key_issue.expire_spend = GNUNET_TIME_absolute_hton (deposit_valid_until); - denom_key_issue.value = TALER_amount_hton (value); - denom_key_issue.fee_withdraw = TALER_amount_hton (fee_withdraw); - denom_key_issue.fee_deposit = TALER_amount_hton (fee_deposit); - denom_key_issue.fee_refresh = TALER_amount_hton (fee_refresh); + TALER_amount_hton (&denom_key_issue.value, + &value); + TALER_amount_hton (&denom_key_issue.fee_withdraw, + &fee_withdraw); + TALER_amount_hton (&denom_key_issue.fee_deposit, + &fee_deposit); + TALER_amount_hton (&denom_key_issue.fee_refresh, + &fee_refresh); EXITIF (GNUNET_SYSERR == GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_MASTER_DENOM, &denom_key_issue.purpose, diff --git a/src/mint/mint_db.c b/src/mint/mint_db.c index 6ed7193e3..545a5252b 100644 --- a/src/mint/mint_db.c +++ b/src/mint/mint_db.c @@ -807,7 +807,8 @@ reserves_update (PGconn *db, TALER_DB_QUERY_PARAM_PTR (&expiry_nbo), TALER_DB_QUERY_PARAM_END }; - balance_nbo = TALER_amount_hton (reserve->balance); + TALER_amount_hton (&balance_nbo, + &reserve->balance); expiry_nbo = GNUNET_TIME_absolute_hton (reserve->expiry); result = TALER_DB_exec_prepared (db, "update_reserve", @@ -837,7 +838,7 @@ reserves_update (PGconn *db, int TALER_MINT_DB_reserves_in_insert (PGconn *db, struct Reserve *reserve, - const struct TALER_Amount balance, + const struct TALER_Amount *balance, const struct GNUNET_TIME_Absolute expiry) { struct TALER_AmountNBO balance_nbo; @@ -862,7 +863,8 @@ TALER_MINT_DB_reserves_in_insert (PGconn *db, TALER_MINT_DB_rollback (db); return GNUNET_SYSERR; } - balance_nbo = TALER_amount_hton (balance); + TALER_amount_hton (&balance_nbo, + balance); expiry_nbo = GNUNET_TIME_absolute_hton (expiry); if (GNUNET_NO == reserve_exists) { @@ -907,19 +909,27 @@ TALER_MINT_DB_reserves_in_insert (PGconn *db, QUERY_ERR (result); goto rollback; } - PQclear (result); result = NULL; + PQclear (result); + result = NULL; if (GNUNET_NO == reserve_exists) { if (GNUNET_OK != TALER_MINT_DB_commit (db)) return GNUNET_SYSERR; - reserve->balance = balance; + reserve->balance = *balance; reserve->expiry = expiry; return GNUNET_OK; } /* Update reserve */ struct Reserve updated_reserve; updated_reserve.pub = reserve->pub; - updated_reserve.balance = TALER_amount_add (reserve->balance, balance); + + if (GNUNET_OK != + TALER_amount_add (&updated_reserve.balance, + &reserve->balance, + balance)) + { + return GNUNET_SYSERR; + } updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry, reserve->expiry); if (GNUNET_OK != reserves_update (db, &updated_reserve)) goto rollback; @@ -1350,7 +1360,8 @@ TALER_MINT_DB_insert_deposit (PGconn *db_conn, GNUNET_CRYPTO_rsa_signature_encode (deposit->coin.denom_sig, &denom_sig_enc); json_wire_enc = json_dumps (deposit->wire, JSON_COMPACT); - amount_nbo = TALER_amount_hton (deposit->amount); + TALER_amount_hton (&amount_nbo, + &deposit->amount); struct TALER_DB_QueryParam params[]= { TALER_DB_QUERY_PARAM_PTR (&deposit->coin.coin_pub), TALER_DB_QUERY_PARAM_PTR_SIZED (denom_pub_enc, denom_pub_enc_size), diff --git a/src/mint/mint_db.h b/src/mint/mint_db.h index 4a9ec1524..9312f548a 100644 --- a/src/mint/mint_db.h +++ b/src/mint/mint_db.h @@ -224,7 +224,7 @@ TALER_MINT_DB_reserve_get (PGconn *db, int TALER_MINT_DB_reserves_in_insert (PGconn *db, struct Reserve *reserve, - const struct TALER_Amount balance, + const struct TALER_Amount *balance, const struct GNUNET_TIME_Absolute expiry); diff --git a/src/mint/taler-mint-httpd_db.c b/src/mint/taler-mint-httpd_db.c index df68b6570..05c2a48a7 100644 --- a/src/mint/taler-mint-httpd_db.c +++ b/src/mint/taler-mint-httpd_db.c @@ -34,26 +34,6 @@ #include "taler-mint-httpd_keystate.h" -/** - * Get an amount in the mint's currency that is zero. - * - * @return zero amount in the mint's currency - */ -static struct TALER_Amount -mint_amount_native_zero () -{ - struct TALER_Amount amount; - - memset (&amount, - 0, - sizeof (amount)); - memcpy (amount.currency, - MINT_CURRENCY, - strlen (MINT_CURRENCY) + 1); - return amount; -} - - /** * Execute a deposit. The validity of the coin and signature * have already been checked. The database must now check that @@ -99,9 +79,12 @@ TALER_MINT_db_execute_deposit (struct MHD_Connection *connection, mks = TALER_MINT_key_state_acquire (); dki = TALER_MINT_get_denom_key (mks, deposit->coin.denom_pub); - value = TALER_amount_ntoh (dki->issue.value); - fee_deposit = TALER_amount_ntoh (dki->issue.fee_deposit); - fee_refresh = TALER_amount_ntoh (dki->issue.fee_refresh); + TALER_amount_ntoh (&value, + &dki->issue.value); + TALER_amount_ntoh (&fee_deposit, + &dki->issue.fee_deposit); + TALER_amount_ntoh (&fee_refresh, + &dki->issue.fee_refresh); TALER_MINT_key_state_release (mks); if (GNUNET_OK != @@ -113,26 +96,49 @@ TALER_MINT_db_execute_deposit (struct MHD_Connection *connection, tl = TALER_MINT_DB_get_coin_transactions (db_conn, &deposit->coin.coin_pub); spent = fee_deposit; /* fee for THIS transaction */ - /* FIXME: need to deal better with integer overflows - in the logic that follows! (change amount.c API! -- #3637) */ - spent = TALER_amount_add (spent, - deposit->amount); + if (GNUNET_OK != + TALER_amount_add (&spent, + &spent, + &deposit->amount)) + { + GNUNET_break (0); + TALER_MINT_DB_free_coin_transaction_list (tl); + return TALER_MINT_reply_internal_db_error (connection); + } for (pos = tl; NULL != pos; pos = pos->next) { switch (pos->type) { case TALER_MINT_DB_TT_DEPOSIT: - spent = TALER_amount_add (spent, - pos->details.deposit->amount); - spent = TALER_amount_add (spent, - fee_deposit); + if ( (GNUNET_OK != + TALER_amount_add (&spent, + &spent, + &pos->details.deposit->amount)) || + (GNUNET_OK != + TALER_amount_add (&spent, + &spent, + &fee_deposit)) ) + { + GNUNET_break (0); + TALER_MINT_DB_free_coin_transaction_list (tl); + return TALER_MINT_reply_internal_db_error (connection); + } break; case TALER_MINT_DB_TT_REFRESH_MELT: - spent = TALER_amount_add (spent, - pos->details.melt->amount); - spent = TALER_amount_add (spent, - fee_refresh); + if ( (GNUNET_OK != + TALER_amount_add (&spent, + &spent, + &pos->details.melt->amount)) || + (GNUNET_OK != + TALER_amount_add (&spent, + &spent, + &fee_refresh)) ) + { + GNUNET_break (0); + TALER_MINT_DB_free_coin_transaction_list (tl); + return TALER_MINT_reply_internal_db_error (connection); + } break; case TALER_MINT_DB_TT_LOCK: /* should check if lock is still active, @@ -146,11 +152,12 @@ TALER_MINT_db_execute_deposit (struct MHD_Connection *connection, } } - if (0 < TALER_amount_cmp (spent, value)) + if (0 < TALER_amount_cmp (&spent, + &value)) { TALER_MINT_DB_rollback (db_conn); ret = TALER_MINT_reply_deposit_insufficient_funds (connection, - tl); + tl); TALER_MINT_DB_free_coin_transaction_list (tl); return ret; } @@ -251,6 +258,7 @@ TALER_MINT_db_execute_withdraw_sign (struct MHD_Connection *connection, struct TALER_Amount withdraw_total; struct TALER_Amount balance; struct TALER_Amount value; + struct TALER_Amount fee_withdraw; struct GNUNET_HashCode h_blind; int res; @@ -318,8 +326,20 @@ TALER_MINT_db_execute_withdraw_sign (struct MHD_Connection *connection, } /* calculate amount required including fees */ - amount_required = TALER_amount_add (TALER_amount_ntoh (dki->issue.value), - TALER_amount_ntoh (dki->issue.fee_withdraw)); + TALER_amount_ntoh (&value, + &dki->issue.value); + TALER_amount_ntoh (&fee_withdraw, + &dki->issue.fee_withdraw); + + if (GNUNET_OK != + TALER_amount_add (&amount_required, + &value, + &fee_withdraw)) + { + TALER_MINT_DB_rollback (db_conn); + TALER_MINT_key_state_release (key_state); + return TALER_MINT_reply_internal_db_error (connection); + } /* calculate balance of the reserve */ res = 0; @@ -331,29 +351,45 @@ TALER_MINT_db_execute_withdraw_sign (struct MHD_Connection *connection, if (0 == (res & 1)) deposit_total = pos->details.bank->amount; else - deposit_total = TALER_amount_add (deposit_total, - pos->details.bank->amount); + if (GNUNET_OK != + TALER_amount_add (&deposit_total, + &deposit_total, + &pos->details.bank->amount)) + { + TALER_MINT_DB_rollback (db_conn); + TALER_MINT_key_state_release (key_state); + return TALER_MINT_reply_internal_db_error (connection); + } res |= 1; break; case TALER_MINT_DB_RO_WITHDRAW_COIN: tdki = TALER_MINT_get_denom_key (key_state, pos->details.withdraw->denom_pub); - value = TALER_amount_ntoh (tdki->issue.value); + TALER_amount_ntoh (&value, + &tdki->issue.value); if (0 == (res & 2)) withdraw_total = value; else - withdraw_total = TALER_amount_add (withdraw_total, - value); + if (GNUNET_OK != + TALER_amount_add (&withdraw_total, + &withdraw_total, + &value)) + { + TALER_MINT_DB_rollback (db_conn); + TALER_MINT_key_state_release (key_state); + return TALER_MINT_reply_internal_db_error (connection); + } res |= 2; break; } } - GNUNET_break (0 > TALER_amount_cmp (withdraw_total, - deposit_total)); - balance = TALER_amount_subtract (deposit_total, - withdraw_total); - if (0 < TALER_amount_cmp (amount_required, - balance)) + /* All reserve balances should be non-negative */ + GNUNET_break (GNUNET_SYSERR != + TALER_amount_subtract (&balance, + &deposit_total, + &withdraw_total)); + if (0 < TALER_amount_cmp (&amount_required, + &balance)) { TALER_MINT_key_state_release (key_state); TALER_MINT_DB_rollback (db_conn); @@ -450,7 +486,8 @@ refresh_accept_melts (struct MHD_Connection *connection, "denom not found")) ? GNUNET_NO : GNUNET_SYSERR; - coin_value = TALER_amount_ntoh (dki->value); + TALER_amount_ntoh (&coin_value, + &dki->value); tl = TALER_MINT_DB_get_coin_transactions (db_conn, &coin_public_info->coin_pub); /* FIXME: #3636: compute how much value is left with this coin and @@ -459,8 +496,8 @@ refresh_accept_melts (struct MHD_Connection *connection, /* Refuse to refresh when the coin does not have enough money left to * pay the refreshing fees of the coin. */ - if (TALER_amount_cmp (coin_residual, - coin_details->melt_amount) < 0) + if (TALER_amount_cmp (&coin_residual, + &coin_details->melt_amount) < 0) { res = (MHD_YES == TALER_MINT_reply_refresh_melt_insufficient_funds (connection, diff --git a/src/mint/taler-mint-httpd_deposit.c b/src/mint/taler-mint-httpd_deposit.c index d37e69e40..37d6d23d5 100644 --- a/src/mint/taler-mint-httpd_deposit.c +++ b/src/mint/taler-mint-httpd_deposit.c @@ -65,7 +65,8 @@ verify_and_execute_deposit (struct MHD_Connection *connection, dr.h_contract = deposit->h_contract; dr.h_wire = deposit->h_wire; dr.transaction_id = GNUNET_htonll (deposit->transaction_id); - dr.amount = TALER_amount_hton (deposit->amount); + TALER_amount_hton (&dr.amount, + &deposit->amount); dr.coin_pub = deposit->coin.coin_pub; if (GNUNET_OK != GNUNET_CRYPTO_ecdsa_verify (TALER_SIGNATURE_WALLET_DEPOSIT, diff --git a/src/mint/taler-mint-httpd_keystate.c b/src/mint/taler-mint-httpd_keystate.c index bf802f5b5..ca802e2d3 100644 --- a/src/mint/taler-mint-httpd_keystate.c +++ b/src/mint/taler-mint-httpd_keystate.c @@ -114,6 +114,19 @@ static json_t * denom_key_issue_to_json (struct GNUNET_CRYPTO_rsa_PublicKey *pk, const struct TALER_MINT_DenomKeyIssue *dki) { + struct TALER_Amount value; + struct TALER_Amount fee_withdraw; + struct TALER_Amount fee_deposit; + struct TALER_Amount fee_refresh; + + TALER_amount_ntoh (&value, + &dki->value); + TALER_amount_ntoh (&fee_withdraw, + &dki->fee_withdraw); + TALER_amount_ntoh (&fee_deposit, + &dki->fee_deposit); + TALER_amount_ntoh (&fee_refresh, + &dki->fee_refresh); return json_pack ("{s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o}", "master_sig", @@ -128,13 +141,13 @@ denom_key_issue_to_json (struct GNUNET_CRYPTO_rsa_PublicKey *pk, "denom_pub", TALER_JSON_from_rsa_public_key (pk), "value", - TALER_JSON_from_amount (TALER_amount_ntoh (dki->value)), + TALER_JSON_from_amount (&value), "fee_withdraw", - TALER_JSON_from_amount (TALER_amount_ntoh (dki->fee_withdraw)), + TALER_JSON_from_amount (&fee_withdraw), "fee_deposit", - TALER_JSON_from_amount (TALER_amount_ntoh (dki->fee_deposit)), + TALER_JSON_from_amount (&fee_deposit), "fee_refresh", - TALER_JSON_from_amount (TALER_amount_ntoh (dki->fee_refresh))); + TALER_JSON_from_amount (&fee_refresh)); } diff --git a/src/mint/taler-mint-httpd_parsing.c b/src/mint/taler-mint-httpd_parsing.c index 6c5f72b32..b8bc043ec 100644 --- a/src/mint/taler-mint-httpd_parsing.c +++ b/src/mint/taler-mint-httpd_parsing.c @@ -878,8 +878,10 @@ TALER_MINT_parse_amount_json (struct MHD_Connection *connection, json_int_t value; json_int_t fraction; const char *currency; - struct TALER_Amount a; + memset (amount, + 0, + sizeof (struct TALER_Amount)); if (-1 == json_unpack (f, "{s:I, s:I, s:s}", "value", &value, @@ -897,7 +899,7 @@ TALER_MINT_parse_amount_json (struct MHD_Connection *connection, } if ( (value < 0) || (fraction < 0) || - (value > UINT32_MAX) || + (value > UINT64_MAX) || (fraction > UINT32_MAX) ) { LOG_WARNING ("Amount specified not in allowed range\n"); @@ -922,11 +924,11 @@ TALER_MINT_parse_amount_json (struct MHD_Connection *connection, return GNUNET_SYSERR; return GNUNET_NO; } - a.value = (uint32_t) value; - a.fraction = (uint32_t) fraction; + amount->value = (uint64_t) value; + amount->fraction = (uint32_t) fraction; GNUNET_assert (strlen (MINT_CURRENCY) < TALER_CURRENCY_LEN); - strcpy (a.currency, MINT_CURRENCY); - *amount = TALER_amount_normalize (a); + strcpy (amount->currency, MINT_CURRENCY); + TALER_amount_normalize (amount); return GNUNET_OK; } diff --git a/src/mint/taler-mint-httpd_refresh.c b/src/mint/taler-mint-httpd_refresh.c index c7bda5a79..e62521ef4 100644 --- a/src/mint/taler-mint-httpd_refresh.c +++ b/src/mint/taler-mint-httpd_refresh.c @@ -82,6 +82,8 @@ handle_refresh_melt_binary (struct MHD_Connection *connection, struct TALER_Amount cost; struct TALER_Amount total_cost; struct TALER_Amount melt; + struct TALER_Amount value; + struct TALER_Amount fee_withdraw; struct TALER_Amount total_melt; /* check that signature from the session public key is ok */ @@ -107,7 +109,8 @@ handle_refresh_melt_binary (struct MHD_Connection *connection, body.purpose.purpose = htonl (TALER_SIGNATURE_REFRESH_MELT_SESSION); body.purpose.size = htonl (sizeof (struct RefreshMeltSessionSignature)); body.melt_hash = melt_hash; - body.amount = TALER_amount_hton (coin_melt_details->melt_amount); + TALER_amount_hton (&body.amount, + &coin_melt_details->melt_amount); if (GNUNET_OK != GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_REFRESH_MELT_SESSION, &body.purpose, @@ -130,11 +133,22 @@ handle_refresh_melt_binary (struct MHD_Connection *connection, { dki = &TALER_MINT_get_denom_key (key_state, denom_pubs[i])->issue; - cost = TALER_amount_add (TALER_amount_ntoh (dki->value), - TALER_amount_ntoh (dki->fee_withdraw)); + TALER_amount_ntoh (&value, + &dki->value); + TALER_amount_ntoh (&fee_withdraw, + &dki->fee_withdraw); // FIXME: #3637 - total_cost = TALER_amount_add (cost, - total_cost); + if ( (GNUNET_OK != + TALER_amount_add (&cost, + &value, + &fee_withdraw)) || + (GNUNET_OK != + TALER_amount_add (&total_cost, + &cost, + &total_cost)) ) + { + // FIXME... + } } // FIXME: badness, use proper way to set to zero... @@ -146,13 +160,18 @@ handle_refresh_melt_binary (struct MHD_Connection *connection, // melt = coin_values[i]; // FIXME: #3636! // FIXME: #3637 - total_melt = TALER_amount_add (melt, - total_melt); + if (GNUNET_OK != + TALER_amount_add (&total_melt, + &melt, + &total_melt)) + { + // FIXME ... + } } TALER_MINT_key_state_release (key_state); if (0 != - TALER_amount_cmp (total_cost, - total_melt) ) + TALER_amount_cmp (&total_cost, + &total_melt) ) { /* We require total value of coins being melted and total value of coins being generated to match! */ @@ -269,7 +288,8 @@ verify_coin_public_info (struct MHD_Connection *connection, body.purpose.size = htonl (sizeof (struct RefreshMeltCoinSignature)); body.purpose.purpose = htonl (TALER_SIGNATURE_REFRESH_MELT_COIN); body.melt_hash = *melt_hash; - body.amount = TALER_amount_hton (r_melt_detail->melt_amount); + TALER_amount_hton (&body.amount, + &r_melt_detail->melt_amount); body.coin_pub = r_public_info->coin_pub; if (GNUNET_OK != GNUNET_CRYPTO_ecdsa_verify (TALER_SIGNATURE_REFRESH_MELT_COIN, diff --git a/src/mint/taler-mint-httpd_responses.c b/src/mint/taler-mint-httpd_responses.c index 3d827b118..a4b442152 100644 --- a/src/mint/taler-mint-httpd_responses.c +++ b/src/mint/taler-mint-httpd_responses.c @@ -299,7 +299,8 @@ TALER_MINT_reply_deposit_success (struct MHD_Connection *connection, dc.h_contract = *h_contract; dc.h_wire = *h_wire; dc.transaction_id = GNUNET_htonll (transaction_id); - dc.amount = TALER_amount_hton (*amount); + TALER_amount_hton (&dc.amount, + amount); dc.coin_pub = *coin_pub; dc.merchant = *merchant; TALER_MINT_keys_sign (&dc.purpose, @@ -346,7 +347,8 @@ compile_transaction_history (const struct TALER_MINT_DB_TransactionList *tl) dr.h_contract = deposit->h_contract; dr.h_wire = deposit->h_wire; dr.transaction_id = GNUNET_htonll (deposit->transaction_id); - dr.amount = TALER_amount_hton (deposit->amount); + TALER_amount_hton (&dr.amount, + &deposit->amount); dr.coin_pub = deposit->coin.coin_pub; transaction = TALER_JSON_from_ecdsa_sig (&dr.purpose, &deposit->csig); @@ -362,7 +364,8 @@ compile_transaction_history (const struct TALER_MINT_DB_TransactionList *tl) ms.purpose.purpose = htonl (TALER_SIGNATURE_REFRESH_MELT_COIN); ms.purpose.size = htonl (sizeof (struct RefreshMeltCoinSignature)); ms.melt_hash = melt->melt_hash; - ms.amount = TALER_amount_hton (melt->amount); + TALER_amount_hton (&ms.amount, + &melt->amount); ms.coin_pub = melt->coin.coin_pub; transaction = TALER_JSON_from_ecdsa_sig (&ms.purpose, &melt->coin_sig); @@ -382,7 +385,7 @@ compile_transaction_history (const struct TALER_MINT_DB_TransactionList *tl) json_array_append_new (history, json_pack ("{s:s, s:o}", "type", type, - "amount", TALER_JSON_from_amount (value), + "amount", TALER_JSON_from_amount (&value), "signature", transaction)); } return history; @@ -419,7 +422,7 @@ TALER_MINT_reply_deposit_insufficient_funds (struct MHD_Connection *connection, * * @param rh reserve history to JSON-ify * @param balance[OUT] set to current reserve balance - * @return json representation of the @a rh + * @return json representation of the @a rh, NULL on error */ static json_t * compile_reserve_history (const struct ReserveHistory *rh, @@ -446,14 +449,20 @@ compile_reserve_history (const struct ReserveHistory *rh, if (0 == ret) deposit_total = pos->details.bank->amount; else - deposit_total = TALER_amount_add (deposit_total, - pos->details.bank->amount); + if (GNUNET_OK != + TALER_amount_add (&deposit_total, + &deposit_total, + &pos->details.bank->amount)) + { + json_decref (json_history); + return NULL; + } ret = 1; json_array_append_new (json_history, json_pack ("{s:s, s:o, s:o}", "type", "DEPOSIT", "wire", pos->details.bank->wire, - "amount", TALER_JSON_from_amount (pos->details.bank->amount))); + "amount", TALER_JSON_from_amount (&pos->details.bank->amount))); break; case TALER_MINT_DB_RO_WITHDRAW_COIN: break; @@ -472,12 +481,20 @@ compile_reserve_history (const struct ReserveHistory *rh, dki = TALER_MINT_get_denom_key (key_state, pos->details.withdraw->denom_pub); - value = TALER_amount_ntoh (dki->issue.value); + TALER_amount_ntoh (&value, + &dki->issue.value); if (0 == ret) withdraw_total = value; else - withdraw_total = TALER_amount_add (withdraw_total, - value); + if (GNUNET_OK != + TALER_amount_add (&withdraw_total, + &withdraw_total, + &value)) + { + TALER_MINT_key_state_release (key_state); + json_decref (json_history); + return NULL; + } ret = 1; wr.purpose.purpose = htonl (TALER_SIGNATURE_WITHDRAW); wr.purpose.size = htonl (sizeof (struct TALER_WithdrawRequest)); @@ -493,15 +510,22 @@ compile_reserve_history (const struct ReserveHistory *rh, json_pack ("{s:s, s:o, s:o}", "type", "WITHDRAW", "signature", transaction, - "amount", TALER_JSON_from_amount (value))); + "amount", TALER_JSON_from_amount (&value))); break; } } TALER_MINT_key_state_release (key_state); - *balance = TALER_amount_subtract (deposit_total, - withdraw_total); + if (GNUNET_SYSERR == + TALER_amount_subtract (balance, + &deposit_total, + &withdraw_total)) + { + GNUNET_break (0); + json_decref (json_history); + return NULL; + } return json_history; } @@ -524,7 +548,10 @@ TALER_MINT_reply_withdraw_status_success (struct MHD_Connection *connection, json_history = compile_reserve_history (rh, &balance); - json_balance = TALER_JSON_from_amount (balance); + if (NULL == json_history) + return TALER_MINT_reply_internal_error (connection, + "balance calculation failure"); + json_balance = TALER_JSON_from_amount (&balance); ret = TALER_MINT_reply_json_pack (connection, MHD_HTTP_OK, "{s:o, s:o}", @@ -556,7 +583,10 @@ TALER_MINT_reply_withdraw_sign_insufficient_funds (struct MHD_Connection *connec json_history = compile_reserve_history (rh, &balance); - json_balance = TALER_JSON_from_amount (balance); + if (NULL == json_history) + return TALER_MINT_reply_internal_error (connection, + "balance calculation failure"); + json_balance = TALER_JSON_from_amount (&balance); ret = TALER_MINT_reply_json_pack (connection, MHD_HTTP_PAYMENT_REQUIRED, "{s:s, s:o, s:o}", @@ -625,9 +655,9 @@ TALER_MINT_reply_refresh_melt_insufficient_funds (struct MHD_Connection *connect "error", "insufficient funds", "coin-pub", TALER_JSON_from_data (coin_pub, sizeof (struct GNUNET_CRYPTO_EcdsaPublicKey)), - "original-value", TALER_JSON_from_amount (coin_value), - "residual-value", TALER_JSON_from_amount (residual), - "requested-value", TALER_JSON_from_amount (requested), + "original-value", TALER_JSON_from_amount (&coin_value), + "residual-value", TALER_JSON_from_amount (&residual), + "requested-value", TALER_JSON_from_amount (&requested), "history", history); } diff --git a/src/mint/taler-mint-keyup.c b/src/mint/taler-mint-keyup.c index 33bb87249..759e7c1b3 100644 --- a/src/mint/taler-mint-keyup.c +++ b/src/mint/taler-mint-keyup.c @@ -233,10 +233,14 @@ hash_coin_type (const struct CoinTypeParams *p, sizeof (struct CoinTypeNBO)); p_nbo.duration_spend = GNUNET_TIME_relative_hton (p->duration_spend); p_nbo.duration_withdraw = GNUNET_TIME_relative_hton (p->duration_withdraw); - p_nbo.value = TALER_amount_hton (p->value); - p_nbo.fee_withdraw = TALER_amount_hton (p->fee_withdraw); - p_nbo.fee_deposit = TALER_amount_hton (p->fee_deposit); - p_nbo.fee_refresh = TALER_amount_hton (p->fee_refresh); + TALER_amount_hton (&p_nbo.value, + &p->value); + TALER_amount_hton (&p_nbo.fee_withdraw, + &p->fee_withdraw); + TALER_amount_hton (&p_nbo.fee_deposit, + &p->fee_deposit); + TALER_amount_hton (&p_nbo.fee_refresh, + &p->fee_refresh); p_nbo.rsa_keysize = htonl (p->rsa_keysize); GNUNET_CRYPTO_hash (&p_nbo, sizeof (struct CoinTypeNBO), @@ -273,7 +277,7 @@ get_cointype_dir (const struct CoinTypeParams *p) GNUNET_assert (HASH_CUTOFF <= strlen (hash_str) + 1); hash_str[HASH_CUTOFF] = 0; - val_str = TALER_amount_to_string (p->value); + val_str = TALER_amount_to_string (&p->value); for (i = 0; i < strlen (val_str); i++) if ( (':' == val_str[i]) || ('.' == val_str[i]) ) @@ -687,11 +691,14 @@ create_denomkey_issue (const struct CoinTypeParams *params, dki->issue.expire_spend = GNUNET_TIME_absolute_hton (GNUNET_TIME_absolute_add (params->anchor, params->duration_spend)); - dki->issue.value = TALER_amount_hton (params->value); - dki->issue.fee_withdraw = TALER_amount_hton (params->fee_withdraw); - dki->issue.fee_deposit = TALER_amount_hton (params->fee_deposit); - dki->issue.fee_refresh = TALER_amount_hton (params->fee_refresh); - + TALER_amount_hton (&dki->issue.value, + ¶ms->value); + TALER_amount_hton (&dki->issue.fee_withdraw, + ¶ms->fee_withdraw); + TALER_amount_hton (&dki->issue.fee_deposit, + ¶ms->fee_deposit); + TALER_amount_hton (&dki->issue.fee_refresh, + ¶ms->fee_refresh); dki->issue.purpose.purpose = htonl (TALER_SIGNATURE_MASTER_DENOM); dki->issue.purpose.size = htonl (sizeof (struct TALER_MINT_DenomKeyIssuePriv) - offsetof (struct TALER_MINT_DenomKeyIssuePriv, diff --git a/src/mint/taler-mint-reservemod.c b/src/mint/taler-mint-reservemod.c index dea59d6ee..df3f0cd5d 100644 --- a/src/mint/taler-mint-reservemod.c +++ b/src/mint/taler-mint-reservemod.c @@ -161,9 +161,11 @@ reservemod_add (struct TALER_Amount denom) "balance_fraction", "balance_currency", &old_denom)); - new_denom = TALER_amount_add (old_denom, - denom); - new_denom_nbo = TALER_amount_hton (new_denom); + TALER_amount_add (&new_denom, + &old_denom, + &denom); + TALER_amount_hton (&new_denom_nbo, + &new_denom); result = PQexecParams (db_conn, "UPDATE reserves" " SET balance_value = $1, balance_fraction = $2, status_sig = NULL, status_sign_pub = NULL" diff --git a/src/util/amount.c b/src/util/amount.c index b3e3b4217..5e7f69fd9 100644 --- a/src/util/amount.c +++ b/src/util/amount.c @@ -30,16 +30,6 @@ #include "taler_util.h" #include -/** - * - */ -#define AMOUNT_FRAC_BASE 1000000 - -/** - * - */ -#define AMOUNT_FRAC_LEN 6 - /** * Parse money amount description, in the format "A:B.C". @@ -53,159 +43,260 @@ int TALER_string_to_amount (const char *str, struct TALER_Amount *denom) { - size_t i; // pos in str - int n; // number tmp - size_t c; // currency pos - uint32_t b; // base for suffix + size_t i; + int n; + uint32_t b; + const char *colon; + const char *value; memset (denom, 0, sizeof (struct TALER_Amount)); + /* skip leading whitespace */ + while (isspace(str[0])) + str++; + if ('\0' == str[0]) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Null before currency\n"); + return GNUNET_SYSERR; + } + /* parse currency */ + colon = strchr (str, (int) ':'); + if ( (NULL == colon) || + ((colon - str) >= TALER_CURRENCY_LEN) ) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Invalid currency specified before colon: `%s'", + str); + goto fail; + } + memcpy (denom->currency, + str, + colon - str); + /* skip colon */ + value = colon + 1; + if ('\0' == value[0]) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Null before value\n"); + goto fail; + } + /* parse value */ i = 0; - while (isspace(str[i])) - i++; - - if (0 == str[i]) + while ('.' != value[i]) { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "null before currency\n"); - return GNUNET_SYSERR; - } - - c = 0; - while (str[i] != ':') - { - if (0 == str[i]) - { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "null before colon"); - return GNUNET_SYSERR; - } - if (c > 3) - { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "currency too long\n"); - return GNUNET_SYSERR; - } - denom->currency[c] = str[i]; - c++; - i++; - } - - // skip colon - i++; - - if (0 == str[i]) - { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "null before value\n"); - return GNUNET_SYSERR; - } - - while (str[i] != '.') - { - if (0 == str[i]) + if ('\0' == value[i]) { return GNUNET_OK; } - n = str[i] - '0'; - if (n < 0 || n > 9) + if ( (str[i] < '0') || (str[i] > '9') ) { GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "invalid character '%c' before comma at %u\n", - (char) n, - i); - return GNUNET_SYSERR; + "Invalid character `%c'\n", + str[i]); + goto fail; + } + n = str[i] - '0'; + if (denom->value * 10 + n < denom->value) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Value too large\n"); + goto fail; } denom->value = (denom->value * 10) + n; i++; } - // skip the dot + /* skip the dot */ i++; - if (0 == str[i]) + /* parse fraction */ + if ('\0' == str[i]) { GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "null after dot"); - return GNUNET_SYSERR; + "Null after dot"); + goto fail; } - - b = 100000; - - while (0 != str[i]) + b = TALER_AMOUNT_FRAC_BASE / 10; + while ('\0' != str[i]) { - n = str[i] - '0'; - if ( (0 == b) || (n < 0) || (n > 9) ) + if (0 == b) { GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "error after comma"); - return GNUNET_SYSERR; + "Fractional value too small (only %u digits supported)", + (unsigned int) TALER_AMOUNT_FRAC_LEN); + goto fail; } + if ( (str[i] < '0') || (str[i] > '9') ) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Error after comma"); + goto fail; + } + n = str[i] - '0'; denom->fraction += n * b; b /= 10; i++; } + return GNUNET_OK; + fail: + /* set currency to 'invalid' to prevent accidental use */ + memset (denom->currency, + 0, + TALER_CURRENCY_LEN); + return GNUNET_SYSERR; +} + + +/** + * Convert amount from host to network representation. + * + * @param res where to store amount in network representation + * @param d amount in host representation + */ +void +TALER_amount_hton (struct TALER_AmountNBO *res, + const struct TALER_Amount *d) +{ + res->value = GNUNET_htonll (d->value); + res->fraction = htonl (d->fraction); + memcpy (res->currency, + d->currency, + TALER_CURRENCY_LEN); +} + + +/** + * Convert amount from network to host representation. + * + * @param res where to store amount in host representation + * @param d amount in network representation + */ +void +TALER_amount_ntoh (struct TALER_Amount *res, + const struct TALER_AmountNBO *dn) +{ + res->value = GNUNET_ntohll (dn->value); + res->fraction = ntohl (dn->fraction); + memcpy (res->currency, + dn->currency, + TALER_CURRENCY_LEN); +} + + +/** + * Get the value of "zero" in a particular currency. + * + * @param cur currency description + * @param denom denomination to write the result to + * @return #GNUNET_OK if @a cur is a valid currency specification, + * #GNUNET_SYSERR if it is invalid. + */ +int +TALER_amount_get_zero (const char *cur, + struct TALER_Amount *denom) +{ + size_t slen; + + slen = strlen (cur); + if (slen >= TALER_CURRENCY_LEN) + return GNUNET_SYSERR; + memset (denom, + 0, + sizeof (struct TALER_Amount)); + memcpy (denom->currency, + cur, + slen); return GNUNET_OK; } /** - * FIXME + * Set @a a to "invalid". + * + * @param a amount to set to invalid */ -struct TALER_AmountNBO -TALER_amount_hton (const struct TALER_Amount d) +static void +invalidate (struct TALER_Amount *a) { - struct TALER_AmountNBO dn; - dn.value = htonl (d.value); - dn.fraction = htonl (d.fraction); - memcpy (dn.currency, d.currency, TALER_CURRENCY_LEN); - - return dn; + memset (a, + 0, + sizeof (struct TALER_Amount)); } /** - * FIXME + * Test if @a a is valid + * + * @param a amount to test + * @return #GNUNET_YES if valid, + * #GNUNET_NO if invalid */ -struct TALER_Amount -TALER_amount_ntoh (const struct TALER_AmountNBO dn) +static int +test_valid (const struct TALER_Amount *a) { - struct TALER_Amount d; - d.value = ntohl (dn.value); - d.fraction = ntohl (dn.fraction); - memcpy (d.currency, dn.currency, TALER_CURRENCY_LEN); - - return d; + return ('\0' != a->currency[0]); } /** - * Compare the value/fraction of two amounts. Does not compare the currency, - * i.e. comparing amounts with the same value and fraction but different - * currency would return 0. + * Test if @a a1 and @a a2 are the same currency. + * + * @param a1 amount to test + * @param a2 amount to test + * @return #GNUNET_YES if @a a1 and @a a2 are the same currency + * #GNUNET_NO if the currencies are different, + * #GNUNET_SYSERR if either amount is invalid + */ +int +TALER_amount_cmp_currency (const struct TALER_Amount *a1, + const struct TALER_Amount *a2) +{ + if ( (GNUNET_NO == test_valid (a1)) || + (GNUNET_NO == test_valid (a2)) ) + return GNUNET_SYSERR; + if (0 == strcmp (a1->currency, + a2->currency)) + return GNUNET_YES; + return GNUNET_NO; +} + + +/** + * Compare the value/fraction of two amounts. Does not compare the currency. + * Comparing amounts of different currencies will cause the program to abort(). + * If unsure, check with #TALER_amount_cmp_currency() first to be sure that + * the currencies of the two amounts are identical. * * @param a1 first amount * @param a2 second amount * @return result of the comparison */ int -TALER_amount_cmp (struct TALER_Amount a1, - struct TALER_Amount a2) +TALER_amount_cmp (const struct TALER_Amount *a1, + const struct TALER_Amount *a2) { - a1 = TALER_amount_normalize (a1); - a2 = TALER_amount_normalize (a2); - if (a1.value == a2.value) + struct TALER_Amount n1; + struct TALER_Amount n2; + + GNUNET_assert (GNUNET_YES == + TALER_amount_cmp_currency (a1, a2)); + n1 = *a1; + n2 = *a2; + TALER_amount_normalize (&n1); + TALER_amount_normalize (&n2); + if (n1.value == n2.value) { - if (a1.fraction < a2.fraction) + if (n1.fraction < n2.fraction) return -1; - if (a1.fraction > a2.fraction) + if (n1.fraction > n2.fraction) return 1; return 0; } - if (a1.value < a2.value) + if (n1.value < n2.value) return -1; return 1; } @@ -214,109 +305,142 @@ TALER_amount_cmp (struct TALER_Amount a1, /** * Perform saturating subtraction of amounts. * + * @param diff where to store (@a a1 - @a a2), or invalid if @a a2 > @a a1 * @param a1 amount to subtract from * @param a2 amount to subtract - * @return (a1-a2) or 0 if a2>=a1 + * @return #GNUNET_OK if the subtraction worked, + * #GNUNET_NO if @a a1 = @a a2 + * #GNUNET_SYSERR if @a a2 > @a a1 or currencies are incompatible; + * @a diff is set to invalid */ -struct TALER_Amount -TALER_amount_subtract (struct TALER_Amount a1, - struct TALER_Amount a2) +int +TALER_amount_subtract (struct TALER_Amount *diff, + const struct TALER_Amount *a1, + const struct TALER_Amount *a2) { - a1 = TALER_amount_normalize (a1); - a2 = TALER_amount_normalize (a2); + struct TALER_Amount n1; + struct TALER_Amount n2; - if (a1.value < a2.value) + if (GNUNET_YES != + TALER_amount_cmp_currency (a1, a2)) { - a1.value = 0; - a1.fraction = 0; - return a1; + invalidate (diff); + return GNUNET_SYSERR; } + n1 = *a1; + n2 = *a2; + TALER_amount_normalize (&n1); + TALER_amount_normalize (&n2); - if (a1.fraction < a2.fraction) + if (n1.fraction < n2.fraction) { - if (0 == a1.value) + if (0 == n1.value) { - a1.fraction = 0; - return a1; + invalidate (diff); + return GNUNET_SYSERR; } - a1.fraction += AMOUNT_FRAC_BASE; - a1.value -= 1; + n1.fraction += TALER_AMOUNT_FRAC_BASE; + n1.value--; } - - a1.fraction -= a2.fraction; - a1.value -= a2.value; - - return a1; + if (n1.value < n2.value) + { + invalidate (diff); + return GNUNET_SYSERR; + } + GNUNET_assert (GNUNET_OK == + TALER_amount_get_zero (a1->currency, + diff)); + GNUNET_assert (n1.fraction >= n2.fraction); + diff->fraction = n1.fraction - n2.fraction; + GNUNET_assert (n1.value >= n2.value); + diff->value = n1.value - n2.value; + if ( (0 == diff->fraction) && + (0 == diff->value) ) + return GNUNET_NO; + return GNUNET_OK; } /** - * Perform saturating addition of amounts. + * Perform addition of amounts. * + * @param sum where to store @a a1 + @a a2, set to "invalid" on overflow * @param a1 first amount to add * @param a2 second amount to add - * @return sum of a1 and a2 + * @return #GNUNET_OK if the addition worked, + * #GNUNET_SYSERR on overflow */ -struct TALER_Amount -TALER_amount_add (struct TALER_Amount a1, - struct TALER_Amount a2) +int +TALER_amount_add (struct TALER_Amount *sum, + const struct TALER_Amount *a1, + const struct TALER_Amount *a2) { - a1 = TALER_amount_normalize (a1); - a2 = TALER_amount_normalize (a2); + struct TALER_Amount n1; + struct TALER_Amount n2; - a1.value += a2.value; - a1.fraction += a2.fraction; - - if (0 == a1.currency[0]) + if (GNUNET_YES != + TALER_amount_cmp_currency (a1, a2)) { - memcpy (a2.currency, - a1.currency, - TALER_CURRENCY_LEN); + invalidate (sum); + return GNUNET_SYSERR; } + n1 = *a1; + n2 = *a2; + TALER_amount_normalize (&n1); + TALER_amount_normalize (&n2); - if (0 == a2.currency[0]) + GNUNET_assert (GNUNET_OK == + TALER_amount_get_zero (a1->currency, + sum)); + sum->value = n1.value + n2.value; + if (sum->value < n1.value) { - memcpy (a1.currency, - a2.currency, - TALER_CURRENCY_LEN); + /* integer overflow */ + invalidate (sum); + return GNUNET_SYSERR; } - - if ( (0 != a1.currency[0]) && - (0 != memcmp (a1.currency, - a2.currency, - TALER_CURRENCY_LEN)) ) + sum->fraction = n1.fraction + n2.fraction; + if (GNUNET_SYSERR == + TALER_amount_normalize (sum)) { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "adding mismatching currencies\n"); + /* integer overflow via carry from fraction */ + invalidate (sum); + return GNUNET_SYSERR; } - - if (a1.value < a2.value) - { - a1.value = UINT32_MAX; - a2.value = UINT32_MAX; - return a1; - } - - return TALER_amount_normalize (a1); + return GNUNET_OK; } /** * Normalize the given amount. * - * @param amout amount to normalize - * @return normalized amount + * @param amount amount to normalize + * @return #GNUNET_OK if normalization worked + * #GNUNET_NO if value was already normalized + * #GNUNET_SYSERR if value was invalid or could not be normalized */ -struct TALER_Amount -TALER_amount_normalize (struct TALER_Amount amount) +int +TALER_amount_normalize (struct TALER_Amount *amount) { - while ( (amount.value != UINT32_MAX) && - (amount.fraction >= AMOUNT_FRAC_BASE) ) + int ret; + + if (GNUNET_YES != test_valid (amount)) + return GNUNET_SYSERR; + ret = GNUNET_NO; + while ( (amount->value != UINT64_MAX) && + (amount->fraction >= TALER_AMOUNT_FRAC_BASE) ) { - amount.fraction -= AMOUNT_FRAC_BASE; - amount.value += 1; + amount->fraction -= TALER_AMOUNT_FRAC_BASE; + amount->value++; + ret = GNUNET_OK; } - return amount; + if (amount->fraction >= TALER_AMOUNT_FRAC_BASE) + { + /* failed to normalize, adding up fractions caused + main value to overflow! */ + return GNUNET_SYSERR; + } + return ret; } @@ -327,40 +451,40 @@ TALER_amount_normalize (struct TALER_Amount amount) * @return freshly allocated string representation */ char * -TALER_amount_to_string (struct TALER_Amount amount) +TALER_amount_to_string (const struct TALER_Amount *amount) { - char tail[AMOUNT_FRAC_LEN + 1] = { 0 }; - char curr[TALER_CURRENCY_LEN + 1] = { 0 }; - char *result = NULL; - int len; + char *result; + uint32_t n; + char tail[TALER_AMOUNT_FRAC_LEN + 1]; + unsigned int i; + struct TALER_Amount norm; - memcpy (curr, amount.currency, TALER_CURRENCY_LEN); - - amount = TALER_amount_normalize (amount); - if (0 != amount.fraction) + if (GNUNET_YES != test_valid (amount)) + return NULL; + norm = *amount; + GNUNET_break (GNUNET_SYSERR != + TALER_amount_normalize (&norm)); + if (0 != (n = norm.fraction)) { - unsigned int i; - uint32_t n = amount.fraction; - for (i = 0; (i < AMOUNT_FRAC_LEN) && (n != 0); i++) + for (i = 0; (i < TALER_AMOUNT_FRAC_LEN) && (0 != n); i++) { - tail[i] = '0' + (n / (AMOUNT_FRAC_BASE / 10)); - n = (n * 10) % (AMOUNT_FRAC_BASE); + tail[i] = '0' + (n / (TALER_AMOUNT_FRAC_BASE / 10)); + n = (n * 10) % (TALER_AMOUNT_FRAC_BASE); } - tail[i] = 0; - len = GNUNET_asprintf (&result, - "%s:%lu.%s", - curr, - (unsigned long) amount.value, - tail); + tail[i] = '\0'; + GNUNET_asprintf (&result, + "%s:%llu.%s", + norm.currency, + (unsigned long long) norm.value, + tail); } else { - len = GNUNET_asprintf (&result, - "%s:%lu", - curr, - (unsigned long) amount.value); + GNUNET_asprintf (&result, + "%s:%llu", + norm.currency, + (unsigned long long) norm.value); } - GNUNET_assert (len > 0); return result; } diff --git a/src/util/json.c b/src/util/json.c index 84fac4c98..7390eb474 100644 --- a/src/util/json.c +++ b/src/util/json.c @@ -48,14 +48,25 @@ * @return a json object describing the amount */ json_t * -TALER_JSON_from_amount (struct TALER_Amount amount) +TALER_JSON_from_amount (const struct TALER_Amount *amount) { json_t *j; - j = json_pack ("{s: s, s:I, s:I}", - "currency", amount.currency, - "value", (json_int_t) amount.value, - "fraction", (json_int_t) amount.fraction); + if ( (amount->value != (uint64_t) ((json_int_t) amount->value)) || + (0 > ((json_int_t) amount->value)) ) + { + /* Theoretically, json_int_t can be a 32-bit "long", or we might + have a 64-bit value which converted to a 63-bit signed long + long causes problems here. So we check. Note that depending + on the platform, the compiler may be able to statically tell + that at least the first check is always false. */ + GNUNET_break (0); + return NULL; + } + j = json_pack ("{s:s, s:I, s:I}", + "currency", amount->currency, + "value", (json_int_t) amount->value, + "fraction", (json_int_t) amount->fraction); GNUNET_assert (NULL != j); return j; }