Skip to content

Commit d96f0eb

Browse files
MiguelCompanyMiguelCompany
committed
Fix CVE-2026-22591
This commit fixes CVE-2026-22591 by limiting the number of subexpressions in a filter expression. This is a squashed commit of a privately reviewed branch. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Reviewed-by: Ricardo González Moreno <ricardo@richiware.dev>
1 parent ce4af9b commit d96f0eb

5 files changed

Lines changed: 423 additions & 41 deletions

File tree

‎src/cpp/fastdds/domain/DomainParticipantImpl.cpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
#include <fastdds/publisher/DataWriterImpl.hpp>
6161
#include <fastdds/subscriber/SubscriberImpl.hpp>
6262
#include <fastdds/topic/ContentFilteredTopicImpl.hpp>
63+
#include <fastdds/topic/DDSSQLFilter/DDSFilterFactory.hpp>
6364
#include <fastdds/topic/TopicImpl.hpp>
6465
#include <fastdds/topic/TopicProxy.hpp>
6566
#include <fastdds/topic/TopicProxyFactory.hpp>
@@ -93,6 +94,54 @@ using fastrtps::rtps::EndpointKind_t;
9394
using fastrtps::rtps::ResourceEvent;
9495
using eprosima::fastdds::dds::Log;
9596

97+
static size_t get_filter_max_subexpressions(
98+
const DomainParticipantQos& qos)
99+
{
100+
constexpr const char parameter_name[] = "dds.sql.expression.max_subexpressions";
101+
const std::string* property = fastrtps::rtps::PropertyPolicyHelper::find_property(
102+
qos.properties(), parameter_name);
103+
if (nullptr != property)
104+
{
105+
try
106+
{
107+
return std::stoul(*property);
108+
}
109+
catch (...)
110+
{
111+
EPROSIMA_LOG_WARNING(DOMAIN_PARTICIPANT,
112+
"Invalid value for dds.sql.expression.max_subexpressions property: "
113+
<< *property << ". Will use default value of "
114+
<< DDSSQLFilter::DDSFilterFactory::DEFAULT_MAX_SUBEXPRESSIONS);
115+
}
116+
}
117+
118+
return DDSSQLFilter::DDSFilterFactory::DEFAULT_MAX_SUBEXPRESSIONS;
119+
}
120+
121+
static size_t get_filter_max_expression_length(
122+
const DomainParticipantQos& qos)
123+
{
124+
constexpr const char parameter_name[] = "dds.sql.expression.max_expression_length";
125+
const std::string* property = fastrtps::rtps::PropertyPolicyHelper::find_property(
126+
qos.properties(), parameter_name);
127+
if (nullptr != property)
128+
{
129+
try
130+
{
131+
return std::stoul(*property);
132+
}
133+
catch (...)
134+
{
135+
EPROSIMA_LOG_WARNING(DOMAIN_PARTICIPANT,
136+
"Invalid value for dds.sql.expression.max_expression_length property: "
137+
<< *property << ". Will use default value of "
138+
<< DDSSQLFilter::DDSFilterFactory::DEFAULT_MAX_EXPRESSION_LENGTH);
139+
}
140+
}
141+
142+
return DDSSQLFilter::DDSFilterFactory::DEFAULT_MAX_EXPRESSION_LENGTH;
143+
}
144+
96145
DomainParticipantImpl::DomainParticipantImpl(
97146
DomainParticipant* dp,
98147
DomainId_t did,
@@ -106,6 +155,7 @@ DomainParticipantImpl::DomainParticipantImpl(
106155
, listener_(listen)
107156
, default_pub_qos_(PUBLISHER_QOS_DEFAULT)
108157
, default_sub_qos_(SUBSCRIBER_QOS_DEFAULT)
158+
, dds_sql_filter_factory_(get_filter_max_subexpressions(qos), get_filter_max_expression_length(qos))
109159
, default_topic_qos_(TOPIC_QOS_DEFAULT)
110160
, id_counter_(0)
111161
#pragma warning (disable : 4355 )

‎src/cpp/fastdds/topic/DDSSQLFilter/DDSFilterFactory.cpp‎

Lines changed: 86 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -521,51 +521,60 @@ IContentFilterFactory::ReturnCode_t DDSFilterFactory::create_content_filter(
521521
}
522522
}
523523
}
524-
else if (std::strlen(filter_expression) == 0)
525-
{
526-
delete_content_filter(filter_class_name, filter_instance);
527-
filter_instance = &empty_expression_;
528-
ret = ReturnCode_t::RETCODE_OK;
529-
}
530524
else
531525
{
532-
auto type_id = get_complete_type_identifier(type_name, data_type);
533-
if (!type_id)
526+
size_t expression_len = std::strlen(filter_expression);
527+
528+
if (0 == expression_len)
534529
{
535-
EPROSIMA_LOG_ERROR(DDSSQLFILTER, "No TypeObject found for type " << type_name);
536-
ret = ReturnCode_t::RETCODE_BAD_PARAMETER;
530+
delete_content_filter(filter_class_name, filter_instance);
531+
filter_instance = &empty_expression_;
532+
ret = ReturnCode_t::RETCODE_OK;
537533
}
538-
else
534+
else if (validate_filter_expression(filter_expression, expression_len))
539535
{
540-
auto type_object = TypeObjectFactory::get_instance()->get_type_object(type_id);
541-
auto node = parser::parse_filter_expression(filter_expression, type_object);
542-
if (node)
536+
auto type_id = get_complete_type_identifier(type_name, data_type);
537+
if (!type_id)
543538
{
544-
auto dyn_type = TypeObjectFactory::get_instance()->build_dynamic_type(type_name, type_id, type_object);
545-
DDSFilterExpression* expr = get_expression();
546-
expr->set_type(dyn_type);
547-
size_t n_params = filter_parameters.length();
548-
expr->parameters.reserve(n_params);
549-
while (expr->parameters.size() < n_params)
550-
{
551-
expr->parameters.emplace_back();
552-
}
553-
ExpressionParsingState state{ type_object, filter_parameters, expr };
554-
ret = convert_tree<DDSFilterCondition>(state, expr->root, *(node->children[0]));
555-
if (ReturnCode_t::RETCODE_OK == ret)
539+
EPROSIMA_LOG_ERROR(DDSSQLFILTER, "No TypeObject found for type " << type_name);
540+
ret = ReturnCode_t::RETCODE_BAD_PARAMETER;
541+
}
542+
else
543+
{
544+
auto type_object = TypeObjectFactory::get_instance()->get_type_object(type_id);
545+
auto node = parser::parse_filter_expression(filter_expression, type_object);
546+
if (node)
556547
{
557-
delete_content_filter(filter_class_name, filter_instance);
558-
filter_instance = expr;
548+
auto dyn_type = TypeObjectFactory::get_instance()->build_dynamic_type(type_name, type_id, type_object);
549+
DDSFilterExpression* expr = get_expression();
550+
expr->set_type(dyn_type);
551+
size_t n_params = filter_parameters.length();
552+
expr->parameters.reserve(n_params);
553+
while (expr->parameters.size() < n_params)
554+
{
555+
expr->parameters.emplace_back();
556+
}
557+
ExpressionParsingState state{ type_object, filter_parameters, expr };
558+
ret = convert_tree<DDSFilterCondition>(state, expr->root, *(node->children[0]));
559+
if (ReturnCode_t::RETCODE_OK == ret)
560+
{
561+
delete_content_filter(filter_class_name, filter_instance);
562+
filter_instance = expr;
563+
}
564+
else
565+
{
566+
delete_content_filter(filter_class_name, expr);
567+
}
559568
}
560569
else
561570
{
562-
delete_content_filter(filter_class_name, expr);
571+
ret = ReturnCode_t::RETCODE_BAD_PARAMETER;
563572
}
564573
}
565-
else
566-
{
567-
ret = ReturnCode_t::RETCODE_BAD_PARAMETER;
568-
}
574+
}
575+
else
576+
{
577+
ret = ReturnCode_t::RETCODE_BAD_PARAMETER;
569578
}
570579
}
571580

@@ -592,6 +601,50 @@ IContentFilterFactory::ReturnCode_t DDSFilterFactory::delete_content_filter(
592601
return ReturnCode_t::RETCODE_OK;
593602
}
594603

604+
bool DDSFilterFactory::validate_filter_expression(
605+
const char* expression,
606+
const size_t expression_len) const
607+
{
608+
if (expression_len > max_expression_length_)
609+
{
610+
// Expression too long
611+
return false;
612+
}
613+
614+
// Check parentheses balance and count
615+
size_t num_parentheses = 0;
616+
size_t pending_size = expression_len;
617+
for (const char* ptr = expression; (pending_size > 0) && (*ptr != '\0'); ++ptr, --pending_size)
618+
{
619+
if (*ptr == '(')
620+
{
621+
++num_parentheses;
622+
623+
if (num_parentheses > max_subexpressions_)
624+
{
625+
// Too many nested parentheses
626+
return false;
627+
}
628+
}
629+
else if (*ptr == ')')
630+
{
631+
if (num_parentheses == 0)
632+
{
633+
// Unmatched closing parenthesis
634+
return false;
635+
}
636+
--num_parentheses;
637+
}
638+
}
639+
if (num_parentheses != 0)
640+
{
641+
// Unmatched opening parenthesis
642+
return false;
643+
}
644+
645+
return true;
646+
}
647+
595648
} // namespace DDSSQLFilter
596649
} // namespace dds
597650
} // namespace fastdds

‎src/cpp/fastdds/topic/DDSSQLFilter/DDSFilterFactory.hpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,17 @@ class DDSFilterFactory final : public IContentFilterFactory
4040

4141
public:
4242

43+
static constexpr size_t DEFAULT_MAX_SUBEXPRESSIONS = 256;
44+
static constexpr size_t DEFAULT_MAX_EXPRESSION_LENGTH = 16 * 1024;
45+
46+
explicit DDSFilterFactory(
47+
size_t max_subexpressions,
48+
size_t max_expression_length)
49+
: max_subexpressions_(max_subexpressions)
50+
, max_expression_length_(max_expression_length)
51+
{
52+
}
53+
4354
~DDSFilterFactory();
4455

4556
ReturnCode_t create_content_filter(
@@ -56,6 +67,20 @@ class DDSFilterFactory final : public IContentFilterFactory
5667

5768
private:
5869

70+
/**
71+
* @brief Validate a DDS-SQL filter expression.
72+
*
73+
* Performs basic validation checks on the expression before passing it to the PEGTL parser.
74+
*
75+
* @param[in] expression The filter expression to validate.
76+
* @param[in] expression_len The length of the filter expression.
77+
*
78+
* @return true if the expression is valid, false otherwise.
79+
*/
80+
bool validate_filter_expression(
81+
const char* expression,
82+
const size_t expression_len) const;
83+
5984
/**
6085
* Retrieve a DDSFilterExpression from the pool.
6186
*
@@ -86,6 +111,10 @@ class DDSFilterFactory final : public IContentFilterFactory
86111
DDSFilterEmptyExpression empty_expression_;
87112
/// Pool of DDSFilterExpression objects
88113
ObjectPool<DDSFilterExpression*> expression_pool_;
114+
/// Maximum number of sub-expressions allowed in a filter expression
115+
const size_t max_subexpressions_ = DEFAULT_MAX_SUBEXPRESSIONS;
116+
/// Maximum length of a filter expression
117+
const size_t max_expression_length_ = DEFAULT_MAX_EXPRESSION_LENGTH;
89118

90119
};
91120

0 commit comments

Comments
 (0)