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.

Sign up for GitHub

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

Jump to bottom

cryptonote_protocol: some stricter checks on object requests #6554

Merged
merged 2 commits into from
May 19, 2020
Merged

cryptonote_protocol: some stricter checks on object requests #6554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f01d529
cryptonote_protocol: reject requests/notifications before handshake
moneromooo-monero May 17, 2020
bbab044
cryptonote_protocol: stricter limit to number of objects requested
moneromooo-monero May 17, 2020
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
3 changes: 2 additions & 1 deletion src/cryptonote_protocol/cryptonote_protocol_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ PUSH_WARNINGS
DISABLE_VS_WARNINGS(4355)

#define LOCALHOST_INT 2130706433
#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 500
#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 100
Copy link
Contributor

Choose a reason for hiding this comment

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

I bet 500 blocks limit works pretty well. Any good reason to change this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To mitigate DoS potential, as above. 100 objects are smaller than 500 (ceteris paribus).

static_assert(CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT >= BLOCKS_SYNCHRONIZING_DEFAULT_COUNT_PRE_V4, "Invalid CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT");

namespace cryptonote
{
Expand Down
20 changes: 20 additions & 0 deletions src/cryptonote_protocol/cryptonote_protocol_handler.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,12 @@ namespace cryptonote
int t_cryptonote_protocol_handler<t_core>::handle_request_fluffy_missing_tx(int command, NOTIFY_REQUEST_FLUFFY_MISSING_TX::request& arg, cryptonote_connection_context& context)
{
MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_FLUFFY_MISSING_TX (" << arg.missing_tx_indices.size() << " txes), block hash " << arg.block_hash);
if (context.m_state == cryptonote_connection_context::state_before_handshake)
{
LOG_ERROR_CCONTEXT("Requested fluffy tx before handshake, dropping connection");
drop_connection(context, false, false);
return 1;
}

std::vector<std::pair<cryptonote::blobdata, block>> local_blocks;
std::vector<cryptonote::blobdata> local_txs;
Expand Down Expand Up @@ -884,6 +890,8 @@ namespace cryptonote
int t_cryptonote_protocol_handler<t_core>::handle_notify_get_txpool_complement(int command, NOTIFY_GET_TXPOOL_COMPLEMENT::request& arg, cryptonote_connection_context& context)
{
MLOG_P2P_MESSAGE("Received NOTIFY_GET_TXPOOL_COMPLEMENT (" << arg.hashes.size() << " txes)");
if(context.m_state != cryptonote_connection_context::state_normal)
return 1;

std::vector<std::pair<cryptonote::blobdata, block>> local_blocks;
std::vector<cryptonote::blobdata> local_txs;
Expand Down Expand Up @@ -987,6 +995,12 @@ namespace cryptonote
template<class t_core>
int t_cryptonote_protocol_handler<t_core>::handle_request_get_objects(int command, NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context)
{
if (context.m_state == cryptonote_connection_context::state_before_handshake)
{
LOG_ERROR_CCONTEXT("Requested objects before handshake, dropping connection");
drop_connection(context, false, false);
return 1;
}
MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_GET_OBJECTS (" << arg.blocks.size() << " blocks)");
if (arg.blocks.size() > CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT)
{
Expand Down Expand Up @@ -1717,6 +1731,12 @@ skip:
int t_cryptonote_protocol_handler<t_core>::handle_request_chain(int command, NOTIFY_REQUEST_CHAIN::request& arg, cryptonote_connection_context& context)
{
MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_CHAIN (" << arg.block_ids.size() << " blocks");
if (context.m_state == cryptonote_connection_context::state_before_handshake)
{
LOG_ERROR_CCONTEXT("Requested chain before handshake, dropping connection");
drop_connection(context, false, false);
return 1;
}
NOTIFY_RESPONSE_CHAIN_ENTRY::request r;
if(!m_core.find_blockchain_supplement(arg.block_ids, !arg.prune, r))
{
Expand Down