From: Vsevolod Stakhov Date: Thu, 11 Jun 2026 22:02:56 +0000 (+0100) Subject: [Fix] css: detect text hidden via overflow clipping, opacity and max-* sizes X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c869ba3ab1ecfe79ddd1689294b2be12217ec980;p=thirdparty%2Frspamd.git [Fix] css: detect text hidden via overflow clipping, opacity and max-* sizes Phishing messages dilute the visible content with hidden ham text using CSS hiding techniques that the parser did not understand: - 'max-width:0; max-height:0; overflow:hidden' was fully ignored as max-width/max-height/overflow were not parsed at all - 'opacity:0' was parsed but the value was silently discarded in compile_to_block - 'height:0' was applied to the block width due to a copy-paste bug, and zero dimensions were never considered by compute_visibility Fixes: - parse max-width/max-height (clamping width/height) and overflow - treat a block with zero height or width and overflow:hidden as invisible, propagating it to descendants via the display value - treat opacity < 0.1 as a hidden display, as descendants cannot reset the ancestor opacity - do not allow a child display value to resurrect content of a hidden ancestor in propagate_block (display:none is not resettable in CSS) - fix the height->width copy-paste bug and a missing break that made the font-size case fall through into the opacity case --- diff --git a/src/libserver/css/css_property.cxx b/src/libserver/css/css_property.cxx index 1557109428..b1446599f4 100644 --- a/src/libserver/css/css_property.cxx +++ b/src/libserver/css/css_property.cxx @@ -31,9 +31,12 @@ constexpr const auto prop_names_map = frozen::make_unordered_map bool + { + return type == css_property_type::PROPERTY_OVERFLOW; + } + auto operator==(const css_property &other) const { return type == other.type; diff --git a/src/libserver/css/css_rule.cxx b/src/libserver/css/css_rule.cxx index 53001e5774..c1315462f7 100644 --- a/src/libserver/css/css_rule.cxx +++ b/src/libserver/css/css_rule.cxx @@ -202,6 +202,24 @@ allowed_property_value(const css_property &prop, const css_consumed_block &parse } } } + if (prop.is_overflow()) { + if (parser_block.is_token()) { + /* A single token */ + const auto &tok = parser_block.get_token_or_empty(); + + if (tok.type == css_parser_token::token_type::ident_token) { + auto sv = tok.get_string_or_default(""); + + /* Distinguish only the clipping values, as they hide + * the content of a zero sized block entirely */ + if (sv == "hidden" || sv == "clip") { + return css_value{css_display_value::DISPLAY_HIDDEN}; + } + + return css_value{css_display_value::DISPLAY_INLINE}; + } + } + } return std::nullopt; } @@ -385,7 +403,8 @@ auto css_declarations_block::merge_block(const css_declarations_block &other, me auto css_declarations_block::compile_to_block(rspamd_mempool_t *pool) const -> rspamd::html::html_block * { auto *block = rspamd_mempool_alloc0_type(pool, rspamd::html::html_block); - auto opacity = -1; + auto opacity = -1.0f; + std::optional height, width, max_height, max_width; const css_rule *font_rule = nullptr, *background_rule = nullptr; for (const auto &rule: rules) { @@ -408,6 +427,7 @@ auto css_declarations_block::compile_to_block(rspamd_mempool_t *pool) const -> r if (fs) { block->set_font_size(fs.value().dim, fs.value().is_percent); } + break; } case css_property_type::PROPERTY_OPACITY: { opacity = vals.back().to_number().value_or(opacity); @@ -429,16 +449,25 @@ auto css_declarations_block::compile_to_block(rspamd_mempool_t *pool) const -> r break; } case css_property_type::PROPERTY_HEIGHT: { - auto w = vals.back().to_dimension(); - if (w) { - block->set_width(w.value().dim, w.value().is_percent); - } + height = vals.back().to_dimension(); break; } case css_property_type::PROPERTY_WIDTH: { - auto h = vals.back().to_dimension(); - if (h) { - block->set_width(h.value().dim, h.value().is_percent); + width = vals.back().to_dimension(); + break; + } + case css_property_type::PROPERTY_MAX_HEIGHT: { + max_height = vals.back().to_dimension(); + break; + } + case css_property_type::PROPERTY_MAX_WIDTH: { + max_width = vals.back().to_dimension(); + break; + } + case css_property_type::PROPERTY_OVERFLOW: { + auto disp = vals.back().to_display(); + if (disp) { + block->set_overflow(disp.value() == css_display_value::DISPLAY_HIDDEN); } break; } @@ -455,6 +484,41 @@ auto css_declarations_block::compile_to_block(rspamd_mempool_t *pool) const -> r } } + /* + * max-height/max-width clamp the corresponding dimension; mixed + * percent/absolute values cannot be compared, so prefer the zero + * limit in that case as it is the only value that matters for + * the visibility detection + */ + auto clamp_dim = [](std::optional dim, + std::optional max_dim) -> std::optional { + if (dim && max_dim) { + if (dim->is_percent == max_dim->is_percent) { + return max_dim->dim < dim->dim ? max_dim : dim; + } + + return max_dim->dim == 0 ? max_dim : dim; + } + + return dim ? dim : max_dim; + }; + + if (auto h = clamp_dim(height, max_height); h) { + block->set_height(h->dim, h->is_percent); + } + if (auto w = clamp_dim(width, max_width); w) { + block->set_width(w->dim, w->is_percent); + } + + if (opacity >= 0 && opacity < 0.1) { + /* + * A (nearly) zero opacity makes the block invisible, and, unlike + * display, it cannot be reset by descendants; reuse the display + * value as it has the desired inheritance semantics + */ + block->set_display(css_display_value::DISPLAY_HIDDEN); + } + /* Optional properties */ if (!(block->fg_color_mask) && font_rule) { auto &vals = font_rule->get_values(); @@ -527,6 +591,96 @@ TEST_SUITE("css") } } + TEST_CASE("hidden blocks are compiled as invisible") + { + /* Real-world hiding techniques: text is removed from the rendered + * view to dilute the visible content (e.g. of a phishing message) */ + const std::vector hidden_styles{ + "display:block; max-width:0; max-height:0; overflow:hidden", + "opacity:0; height:0; line-height:0; overflow:hidden", + "overflow:hidden; max-height:0", + "height:0px; overflow:clip", + "width:0; overflow:hidden", + "opacity:0.01", + "max-height:0; height:50px; overflow:hidden", + }; + const std::vector visible_styles{ + "max-width:600px; width:100%", + /* Without overflow:hidden the content of a zero sized block + * is rendered outside of the block */ + "height:0", + "max-height:0", + "opacity:0.5", + "overflow:hidden; max-height:10px", + "overflow:hidden", + "display:block", + }; + + auto *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "css", 0); + + for (const auto *css_text: hidden_styles) { + auto res = process_declaration_tokens(pool, + get_rules_parser_functor(pool, css_text)); + CHECK(res.get() != nullptr); + auto *block = res->compile_to_block(pool); + CHECK(block != nullptr); + block->compute_visibility(); + CHECK_MESSAGE(!block->is_visible(), css_text); + } + + for (const auto *css_text: visible_styles) { + auto res = process_declaration_tokens(pool, + get_rules_parser_functor(pool, css_text)); + CHECK(res.get() != nullptr); + auto *block = res->compile_to_block(pool); + CHECK(block != nullptr); + block->compute_visibility(); + CHECK_MESSAGE(block->is_visible(), css_text); + } + + rspamd_mempool_delete(pool); + } + + TEST_CASE("height and width are applied to the proper fields") + { + auto *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "css", 0); + + auto res = process_declaration_tokens(pool, + get_rules_parser_functor(pool, "height:10px;width:20px")); + CHECK(res.get() != nullptr); + auto *block = res->compile_to_block(pool); + CHECK(block != nullptr); + CHECK(static_cast(block->height_mask) != 0); + CHECK(static_cast(block->width_mask) != 0); + CHECK(block->height == 10); + CHECK(block->width == 20); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("hidden parent cannot be reset by child display") + { + auto *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "css", 0); + + auto parent_res = process_declaration_tokens(pool, + get_rules_parser_functor(pool, "display:none")); + auto *parent = parent_res->compile_to_block(pool); + parent->compute_visibility(); + CHECK(!parent->is_visible()); + + auto child_res = process_declaration_tokens(pool, + get_rules_parser_functor(pool, "display:block")); + auto *child = child_res->compile_to_block(pool); + child->propagate_block(*parent); + child->compute_visibility(); + CHECK(!child->is_visible()); + + rspamd_mempool_delete(pool); + } + TEST_CASE("duplicate color properties - last wins") { /* Test case: duplicate color declarations should use the last value diff --git a/src/libserver/html/html_block.hxx b/src/libserver/html/html_block.hxx index f9b5184722..bcae5dfab0 100644 --- a/src/libserver/html/html_block.hxx +++ b/src/libserver/html/html_block.hxx @@ -32,6 +32,7 @@ struct html_block { std::int16_t width; rspamd::css::css_display_value display; std::int8_t font_size; + bool overflow_hidden; unsigned fg_color_mask : 2; unsigned bg_color_mask : 2; @@ -40,6 +41,7 @@ struct html_block { unsigned font_mask : 2; unsigned display_mask : 2; unsigned visibility_mask : 2; + unsigned overflow_mask : 2; constexpr static const auto unset = 0; constexpr static const auto inherited = 1; @@ -107,6 +109,12 @@ struct html_block { display_mask = how; } + auto set_overflow(bool hidden, int how = html_block::set) -> void + { + overflow_hidden = hidden; + overflow_mask = how; + } + auto set_font_size(float fs, bool is_percent = false, int how = html_block::set) -> void { fs = is_percent ? (-fs) : fs; @@ -192,8 +200,20 @@ public: fg_color, other.fg_color); bg_color_mask = html_block::simple_prop(bg_color_mask, other.bg_color_mask, bg_color, other.bg_color); - display_mask = html_block::simple_prop(display_mask, other.display_mask, - display, other.display); + + if (other.display_mask && other.display == css::css_display_value::DISPLAY_HIDDEN) { + /* + * A hidden ancestor hides all descendants: a child cannot reset + * display of a display:none parent, so ignore the own display + * value in this case + */ + display = css::css_display_value::DISPLAY_HIDDEN; + display_mask = html_block::inherited; + } + else { + display_mask = html_block::simple_prop(display_mask, other.display_mask, + display, other.display); + } height_mask = html_block::size_prop(height_mask, other.height_mask, height, other.height, static_cast(800)); @@ -224,6 +244,7 @@ public: height_mask = set_value(height_mask, other.height_mask, height, other.height); width_mask = set_value(width_mask, other.width_mask, width, other.width); font_mask = set_value(font_mask, other.font_mask, font_size, other.font_size); + overflow_mask = set_value(overflow_mask, other.overflow_mask, overflow_hidden, other.overflow_hidden); } auto compute_visibility(void) -> void @@ -244,6 +265,21 @@ public: } } + if (overflow_mask && overflow_hidden) { + /* Zero height or width with overflow:hidden clips the content entirely */ + if ((height_mask && height == 0) || (width_mask && width == 0)) { + /* + * Convert to hidden display as well: clipping cannot be resurrected + * by descendants, so it should be propagated to children as + * the display value + */ + set_display(css::css_display_value::DISPLAY_HIDDEN); + visibility_mask = html_block::invisible_flag; + + return; + } + } + auto is_similar_colors = [](const rspamd::css::css_color &fg, const rspamd::css::css_color &bg) -> bool { constexpr const auto min_visible_diff = 0.1f; auto diff_r = ((float) fg.r - bg.r); @@ -332,13 +368,15 @@ public: .width = 0, .display = rspamd::css::css_display_value::DISPLAY_INLINE, .font_size = 12, + .overflow_hidden = false, .fg_color_mask = html_block::inherited, .bg_color_mask = html_block::inherited, .height_mask = html_block::unset, .width_mask = html_block::unset, .font_mask = html_block::unset, .display_mask = html_block::inherited, - .visibility_mask = html_block::unset}; + .visibility_mask = html_block::unset, + .overflow_mask = html_block::unset}; } /** * Produces html block with no defined values allocated from the pool