sphinx: don't leak, especially on failed onion.

Generally, the pattern is: everything returned is allocated off the return
value, which is the only thing allocated off the context.  And it's always
freed.

Also, tal_free() returns NULL, so it's useful for one-line error
cleanups.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2016-11-04 11:17:04 +10:30
parent de2ffd8985
commit a902193874
2 changed files with 15 additions and 15 deletions

View File

@ -546,20 +546,20 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
packet = parse_onionpacket(peer, peer->dstate->secpctx,
htlc->routing, tal_count(htlc->routing));
if (packet)
step = process_onionpacket(peer, peer->dstate->secpctx, packet, &pk);
step = process_onionpacket(packet, peer->dstate->secpctx, packet, &pk);
if (!step) {
log_unusual(peer->log, "Bad onion, failing HTLC %"PRIu64,
htlc->id);
command_htlc_set_fail(peer, htlc, BAD_REQUEST_400,
"invalid onion");
return;
goto free_packet;
}
switch (step->nextcase) {
case ONION_END:
if (only_dest)
return;
goto free_packet;
invoice = find_unpaid(peer->dstate, &htlc->rhash);
if (!invoice) {
log_unusual(peer->log, "No invoice for HTLC %"PRIu64,
@ -570,7 +570,7 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
command_htlc_set_fail(peer, htlc,
UNAUTHORIZED_401,
"unknown rhash");
goto free_rest;
goto free_packet;
}
if (htlc->msatoshi != invoice->msatoshi) {
@ -583,7 +583,7 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
command_htlc_set_fail(peer, htlc,
UNAUTHORIZED_401,
"incorrect amount");
return;
goto free_packet;
}
log_info(peer->log, "Immediately resolving '%s' HTLC %"PRIu64,
@ -592,22 +592,22 @@ static void their_htlc_added(struct peer *peer, struct htlc *htlc,
resolve_invoice(peer->dstate, invoice);
set_htlc_rval(peer, htlc, &invoice->r);
command_htlc_fulfill(peer, htlc);
goto free_rest;
goto free_packet;
case ONION_FORWARD:
printf("FORWARDING %lu\n", step->hoppayload->amount);
route_htlc_onwards(peer, htlc, step->hoppayload->amount, step->next->nexthop,
serialize_onionpacket(step, peer->dstate->secpctx, step->next), only_dest);
goto free_rest;
goto free_packet;
default:
log_info(peer->log, "Unknown step type %u", step->nextcase);
command_htlc_set_fail(peer, htlc, VERSION_NOT_SUPPORTED_505,
"unknown step type");
goto free_rest;
goto free_packet;
}
free_rest:
tal_free(step);
free_packet:
tal_free(packet);
}
static void our_htlc_failed(struct peer *peer, struct htlc *htlc)

View File

@ -91,12 +91,12 @@ struct onionpacket *parse_onionpacket(
read_buffer(&m->version, src, 1, &p);
if (m->version != 0x01) {
// FIXME add logging
return NULL;
return tal_free(m);
}
read_buffer(rawEphemeralkey, src, 33, &p);
if (secp256k1_ec_pubkey_parse(secpctx, &m->ephemeralkey, rawEphemeralkey, 33) != 1)
return NULL;
return tal_free(m);
read_buffer(&m->mac, src, 20, &p);
read_buffer(&m->routinginfo, src, ROUTING_INFO_SIZE, &p);
@ -478,7 +478,7 @@ struct route_step *process_onionpacket(
u8 stream[NUM_STREAM_BYTES];
u8 paddedheader[ROUTING_INFO_SIZE + 2 * SECURITY_PARAMETER];
step->next = talz(ctx, struct onionpacket);
step->next = talz(step, struct onionpacket);
step->next->version = msg->version;
create_shared_secret(secpctx, secret, &msg->ephemeralkey, hop_privkey->secret);
generate_key_set(secret, &keys);
@ -487,7 +487,7 @@ struct route_step *process_onionpacket(
if (memcmp(msg->mac, hmac, sizeof(hmac)) != 0) {
warnx("Computed MAC does not match expected MAC, the message was modified.");
return NULL;
return tal_free(step);
}
//FIXME:store seen secrets to avoid replay attacks
@ -509,7 +509,7 @@ struct route_step *process_onionpacket(
compute_blinding_factor(secpctx, &msg->ephemeralkey, secret, blind);
if (!blind_group_element(secpctx, &step->next->ephemeralkey, &msg->ephemeralkey, blind))
return NULL;
return tal_free(step);
memcpy(&step->next->nexthop, paddedheader, SECURITY_PARAMETER);
memcpy(&step->next->mac,
paddedheader + SECURITY_PARAMETER,