lib: Enforce a maximum length of input messages

This commit is the 1/3 part of the fix for the following security
vulnerability:
 - CVE-2021-42076 DoS via excess length messages

The issue has been reported by Matthias Gerstner <mgerstner@suse.de>.
This commit is contained in:
Povilas Kanapickas 2021-11-01 05:18:51 +02:00
parent cc369820d4
commit e33c81b835
5 changed files with 52 additions and 11 deletions

View File

@ -0,0 +1,6 @@
SECURITY ISSUE
Barrier will now enforce a maximum length of input messages (fixes CVE-2021-42076).
Previously it was possible for a malicious client or server to send excessive length messages
leading to denial of service by resource exhaustion.

View File

@ -19,6 +19,8 @@
#include "barrier/ProtocolUtil.h"
#include "io/IStream.h"
#include "base/Log.h"
#include "barrier/protocol_types.h"
#include "barrier/XBarrier.h"
#include "common/stdvector.h"
#include "base/String.h"
@ -159,6 +161,10 @@ ProtocolUtil::vreadf(barrier::IStream* stream, const char* fmt, va_list args)
(static_cast<UInt32>(buffer[2]) << 8) |
static_cast<UInt32>(buffer[3]);
if (n > PROTOCOL_MAX_LIST_LENGTH) {
throw XBadClient("Too long message received");
}
// convert it
void* v = va_arg(args, void*);
switch (len) {
@ -211,6 +217,10 @@ ProtocolUtil::vreadf(barrier::IStream* stream, const char* fmt, va_list args)
(static_cast<UInt32>(buffer[2]) << 8) |
static_cast<UInt32>(buffer[3]);
if (len > PROTOCOL_MAX_STRING_LENGTH) {
throw XBadClient("Too long message received");
}
// use a fixed size buffer if its big enough
const bool useFixed = (len <= sizeof(buffer));

View File

@ -51,6 +51,11 @@ static const double kKeepAlivesUntilDeath = 3.0;
static const double kHeartRate = -1.0;
static const double kHeartBeatsUntilDeath = 3.0;
// Messages of very large size indicate a likely protocol error. We don't parse such messages and
// drop connection instead. Note that e.g. the clipboard messages are already limited to 32kB.
static constexpr std::uint32_t PROTOCOL_MAX_LIST_LENGTH = 1024 * 1024;
static constexpr std::uint32_t PROTOCOL_MAX_STRING_LENGTH = 1024 * 1024;
// direction constants
enum EDirection {
kNoDirection,

View File

@ -26,6 +26,7 @@
#include "barrier/ProtocolUtil.h"
#include "barrier/option_types.h"
#include "barrier/protocol_types.h"
#include "barrier/XBarrier.h"
#include "io/IStream.h"
#include "base/Log.h"
#include "base/IEventQueue.h"
@ -124,6 +125,7 @@ ServerProxy::handleData(const Event&, void*)
// parse message
LOG((CLOG_DEBUG2 "msg from server: %c%c%c%c", code[0], code[1], code[2], code[3]));
try {
switch ((this->*m_parser)(code)) {
case kOkay:
break;
@ -136,6 +138,15 @@ ServerProxy::handleData(const Event&, void*)
case kDisconnect:
return;
}
} catch (const XBadClient& e) {
// TODO: disconnect handling is currently dispersed across both parseMessage() and
// handleData() functions, we should collect that to a single place
LOG((CLOG_ERR "protocol error from server: %s", e.what()));
ProtocolUtil::writef(m_stream, kMsgEBad);
m_client->disconnect("invalid message from server");
return;
}
// next message
n = m_stream->read(code, 4);

View File

@ -148,12 +148,21 @@ ClientProxy1_0::handleData(const Event&, void*)
}
// parse message
try {
LOG((CLOG_DEBUG2 "msg from \"%s\": %c%c%c%c", getName().c_str(), code[0], code[1], code[2], code[3]));
if (!(this->*m_parser)(code)) {
LOG((CLOG_ERR "invalid message from client \"%s\": %c%c%c%c", getName().c_str(), code[0], code[1], code[2], code[3]));
disconnect();
return;
}
} catch (const XBadClient& e) {
// TODO: disconnect handling is currently dispersed across both parseMessage() and
// handleData() functions, we should collect that to a single place
LOG((CLOG_ERR "protocol error from client: %s", e.what()));
disconnect();
return;
}
// next message
n = getStream()->read(code, 4);