Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More bolt11 fixes #6776

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/bech32.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
#include <common/bech32.h>
#include <string.h>

#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
bool dev_bech32_nocsum;
#endif

static uint32_t bech32_polymod_step(uint32_t pre) {
uint8_t b = pre >> 25;
return ((pre & 0x1FFFFFF) << 5) ^
Expand Down Expand Up @@ -142,6 +146,13 @@ bech32_encoding bech32_decode(char* hrp, uint8_t *data, size_t *data_len, const
}
++i;
}

#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
if (dev_bech32_nocsum)
/* Skip checksum and upper/lowercase checks when fuzzing. */
return BECH32_ENCODING_BECH32;
#endif

if (have_lower && have_upper) {
return BECH32_ENCODING_NONE;
}
Expand Down
6 changes: 6 additions & 0 deletions common/bech32.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define LIGHTNING_COMMON_BECH32_H
#include "config.h"

#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>

Expand Down Expand Up @@ -130,5 +131,10 @@ int bech32_convert_bits(uint8_t* out, size_t* outlen, int outbits,
extern const char bech32_charset[32];
extern const int8_t bech32_charset_rev[128];

/* Global to weaken csum checks for fuzzing. */
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
extern bool dev_bech32_nocsum;
#endif

#endif /* LIGHTNING_COMMON_BECH32_H */

31 changes: 27 additions & 4 deletions common/bolt11.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ static const char *pull_uint(struct hash_u5 *hu5,
err = pull_bits(hu5, data, data_len, &be_val, databits, true);
if (err)
return err;
*val = be64_to_cpu(be_val) >> (sizeof(be_val) * CHAR_BIT - databits);
if (databits == 0)
*val = 0;
else
*val = be64_to_cpu(be_val) >>
(sizeof(be_val) * CHAR_BIT - databits);
return NULL;
}

Expand Down Expand Up @@ -302,14 +306,26 @@ static const char *decode_n(struct bolt11 *b11,
const u5 **data, size_t *field_len,
bool *have_n)
{
const char *err;

assert(!*have_n);
/* BOLT #11:
*
* A reader... MUST skip over unknown fields, OR an `f` field
* with unknown `version`, OR `p`, `h`, `s` or `n` fields that do
* NOT have `data_length`s of 52, 52, 52 or 53, respectively. */
return pull_expected_length(b11, hu5, data, field_len, 53, 'n',
have_n, &b11->receiver_id.k);
err = pull_expected_length(b11, hu5, data, field_len, 53, 'n',
have_n, &b11->receiver_id.k);

/* If that gave us nodeid, check it. */
if (*have_n) {
struct pubkey k;
if (!pubkey_from_node_id(&k, &b11->receiver_id))
return tal_fmt(b11, "invalid public key %s",
node_id_to_hexstr(tmpctx, &b11->receiver_id));
}

return err;
}

/* BOLT #11:
Expand Down Expand Up @@ -390,6 +406,8 @@ static const char *decode_f(struct bolt11 *b11,
fallback = scriptpubkey_p2sh_hash(b11, shash);
} else if (version < 17) {
u8 *f = pull_all(tmpctx, hu5, data, field_len, false, &err);
if (!f)
return err;
if (version == 0) {
if (tal_count(f) != 20 && tal_count(f) != 32)
return tal_fmt(b11,
Expand Down Expand Up @@ -715,7 +733,6 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
memset(have_field, 0, sizeof(have_field));
b11->routes = tal_arr(b11, struct route_info *, 0);

assert(!has_lightning_prefix(str));
vincenzopalazzo marked this conversation as resolved.
Show resolved Hide resolved
if (strlen(str) < 8)
return decode_fail(b11, fail, "Bad bech32 string");

Expand Down Expand Up @@ -918,6 +935,8 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
return b11;
}

static bool valid_recovery_id(u8 recid) { return recid <= 3; }

/* Decodes and checks signature; returns NULL on error. */
struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
const struct feature_set *our_features,
Expand Down Expand Up @@ -958,6 +977,10 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,

assert(data_len == 0);

if (!valid_recovery_id(sig_and_recid[64]))
return decode_fail(b11, fail, "invalid recovery ID: %u",
sig_and_recid[64]);

if (!secp256k1_ecdsa_recoverable_signature_parse_compact
(secp256k1_ctx, &sig, sig_and_recid, sig_and_recid[64]))
return decode_fail(b11, fail, "signature invalid");
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lnltc1Uggzzzzfzzffffffffffffffffffffffffffffffgfffffffffffffffffzzzzfzzfffffffffffffffffffffffffffffffffffffffffffffffffffff
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lntltc1UZZZZQQDQQpp5pppppppppppppppppZZZZZZZZZZZZQQQQQQQQQQQQQQQQQQQQQQQQQQQQQAQQQQQQQQQQQQQQQQZZZZZZZZZZZZZZZZZZZZZZZppppppppppppppppppppppppppppppppppppZZZZZZZZZZZZZZZZZZZZZZZZtltc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lightning:
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lnltc1zzzzzAzcQQQQQZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZXZZZZZZZZZZZZZZZZZJZZZZZZZZZZzzzZZZZZZZZ
26 changes: 26 additions & 0 deletions tests/fuzz/fuzz-bolt11-decode.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include "config.h"
#include <assert.h>

#include <common/bolt11.h>
#include <common/setup.h>
#include <common/utils.h>
#include <stdio.h>
#include <string.h>
#include <tests/fuzz/libfuzz.h>

void init(int *argc, char ***argv) { common_setup("fuzzer"); }

void run(const u8 *data, size_t size)
{
char *str = tal_arr(NULL, char, size + 1);
char *fail_reason;

memcpy(str, data, size);
str[size] = '\0';

bolt11_decode(str, str, /*our_features=*/NULL, /*description=*/NULL,
/*must_be_chain=*/NULL, &fail_reason);

clean_tmpctx();
tal_free(str);
}
2 changes: 2 additions & 0 deletions tests/fuzz/fuzz-bolt11.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ void run(const uint8_t *data, size_t size)
const u5 *sigdata;
bool have_n = false;
char *fail;

dev_bech32_nocsum = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong fuzz test to enable the flag. We should do this during init in fuzz-bolt11-decode.c instead.

bolt11_decode_nosig(tmpctx, invoice_str, NULL, NULL, chainparams, &hash,
&sigdata, &have_n, &fail);

Expand Down
Loading