Skip to content

Fix buffer overflow if output iterator is std::back_insert_iterator and value is escaped (debug format) - #3797

Merged
vitaut merged 1 commit into
fmtlib:masterfrom
phprus:issue-3795
Jan 10, 2024
Merged

vitaut merged 1 commit into
fmtlib:masterfrom
phprus:issue-3795

Conversation

@phprus

@phprus phprus commented Jan 10, 2024

Copy link
Copy Markdown
Contributor

Issue: #3795

For std::back_insert_iterator output iterator reserve return raw pointer:

fmt/include/fmt/format.h

Lines 571 to 582 in 58a6bd4

template <typename Container, FMT_ENABLE_IF(is_contiguous<Container>::value)>
#if FMT_CLANG_VERSION >= 307 && !FMT_ICC_VERSION
__attribute__((no_sanitize("undefined")))
#endif
inline auto
reserve(std::back_insert_iterator<Container> it, size_t n) ->
typename Container::value_type* {
Container& c = get_container(it);
size_t size = c.size();
c.resize(size + n);
return get_data(c) + size;
}

If size is incorrect, write_padded may overflow this container:

fmt/include/fmt/format.h

Lines 1798 to 1818 in 58a6bd4

// Writes the output of f, padded according to format specifications in specs.
// size: output size in code units.
// width: output display width in (terminal) column positions.
template <align::type align = align::left, typename OutputIt, typename Char,
typename F>
FMT_CONSTEXPR auto write_padded(OutputIt out, const format_specs<Char>& specs,
size_t size, size_t width, F&& f) -> OutputIt {
static_assert(align == align::left || align == align::right, "");
unsigned spec_width = to_unsigned(specs.width);
size_t padding = spec_width > width ? spec_width - width : 0;
// Shifts are encoded as string literals because static constexpr is not
// supported in constexpr functions.
auto* shifts = align == align::left ? "\x1f\x1f\x00\x01" : "\x00\x1f\x00\x01";
size_t left_padding = padding >> shifts[specs.align];
size_t right_padding = padding - left_padding;
auto it = reserve(out, size + padding * specs.fill.size());
if (left_padding != 0) it = fill(it, left_padding, specs.fill);
it = f(it);
if (right_padding != 0) it = fill(it, right_padding, specs.fill);
return base_iterator(out, it);
}

@vitaut

@phprus

phprus commented Jan 10, 2024

Copy link
Copy Markdown
Contributor Author

Format error in file include/fmt/base.h

@vitaut
I suggest enabling lint for all commits, not just PRs.

@vitaut

vitaut commented Jan 10, 2024

Copy link
Copy Markdown
Contributor

Format error in file include/fmt/base.h

Should be fixed now.

I suggest enabling lint for all commits, not just PRs.

Yeah, we should do that.

…nd value is escaped (debug format)

Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
@phprus

phprus commented Jan 10, 2024

Copy link
Copy Markdown
Contributor Author

@vitaut
Rebased.

@vitaut
vitaut merged commit 961df82 into fmtlib:master Jan 10, 2024
@vitaut

vitaut commented Jan 10, 2024

Copy link
Copy Markdown
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants