Skip to content

Commit 82a685f

Browse files
committed
[BACKPORT] Reject illegal paths in OSC 9;9 (#14093)
Paths that contain illegal path components will be dropped in the dispatcher. Service-Version: 1.15 (cherry picked from commit fc0ef37) Service-Version: 1.12 (cherry picked from commit a187987)
1 parent 9bda600 commit 82a685f

File tree

4 files changed

+90
-20
lines changed

4 files changed

+90
-20
lines changed

src/cascadia/TerminalCore/TerminalDispatch.cpp

+11-7
Original file line numberDiff line numberDiff line change
@@ -542,19 +542,23 @@ bool TerminalDispatch::DoConEmuAction(const std::wstring_view string) noexcept
542542
{
543543
if (parts.size() >= 2)
544544
{
545-
const auto path = til::at(parts, 1);
545+
auto path = til::at(parts, 1);
546546
// The path should be surrounded with '"' according to the documentation of ConEmu.
547547
// An example: 9;"D:/"
548-
if (path.at(0) == L'"' && path.at(path.size() - 1) == L'"' && path.size() >= 3)
548+
// If we fail to find the surrounding quotation marks, we'll give the path a try anyway.
549+
// ConEmu also does this.
550+
if (path.size() >= 3 && path.at(0) == L'"' && path.at(path.size() - 1) == L'"')
549551
{
550-
return _terminalApi.SetWorkingDirectory(path.substr(1, path.size() - 2));
552+
path = path.substr(1, path.size() - 2);
551553
}
552-
else
554+
555+
if (!til::is_legal_path(path))
553556
{
554-
// If we fail to find the surrounding quotation marks, we'll give the path a try anyway.
555-
// ConEmu also does this.
556-
return _terminalApi.SetWorkingDirectory(path);
557+
return false;
557558
}
559+
560+
_terminalApi.SetWorkingDirectory(path);
561+
return true;
558562
}
559563
}
560564

src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp

+21-13
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,27 @@ void TerminalCoreUnitTests::TerminalApiTest::SetWorkingDirectory()
436436
stateMachine.ProcessString(L"\x1b]9;9\"C:\\\"\x9c");
437437
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
438438

439+
stateMachine.ProcessString(L"\x1b"
440+
LR"(]9;9;"C:\invalid path "with" quotes")"
441+
L"\x1b\\");
442+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
443+
444+
// These OSC 9;9 sequences will result in invalid CWD. It should end up empty, like above.
445+
stateMachine.ProcessString(L"\x1b]9;9;\"\x1b\\");
446+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
447+
448+
stateMachine.ProcessString(L"\x1b]9;9;\"\"\x1b\\");
449+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
450+
451+
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\x1b\\");
452+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
453+
454+
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\"\x1b\\");
455+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
456+
457+
stateMachine.ProcessString(L"\x1b]9;9;No quotes \"until\" later\x1b\\");
458+
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());
459+
439460
// Valid sequences should change CWD
440461
stateMachine.ProcessString(L"\x1b]9;9;\"C:\\\"\x9c");
441462
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"C:\\");
@@ -455,17 +476,4 @@ void TerminalCoreUnitTests::TerminalApiTest::SetWorkingDirectory()
455476

456477
stateMachine.ProcessString(L"\x1b]9;9;D:\\中文\x9c");
457478
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"D:\\中文");
458-
459-
// These OSC 9;9 sequences will result in invalid CWD. We shouldn't crash on these.
460-
stateMachine.ProcessString(L"\x1b]9;9;\"\x9c");
461-
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"");
462-
463-
stateMachine.ProcessString(L"\x1b]9;9;\"\"\x9c");
464-
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"\"");
465-
466-
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\x9c");
467-
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"");
468-
469-
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\"\x9c");
470-
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"\"");
471479
}

src/inc/til/string.h

+52
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,58 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
3030
return visualize_control_codes(std::wstring{ str });
3131
}
3232

33+
namespace details
34+
{
35+
inline constexpr uint8_t __ = 0b00;
36+
inline constexpr uint8_t F_ = 0b10; // stripped in clean_filename
37+
inline constexpr uint8_t _P = 0b01; // stripped in clean_path
38+
inline constexpr uint8_t FP = 0b11; // stripped in clean_filename and clean_path
39+
static constexpr std::array<uint8_t, 128> pathFilter{ {
40+
// clang-format off
41+
__ /* NUL */, __ /* SOH */, __ /* STX */, __ /* ETX */, __ /* EOT */, __ /* ENQ */, __ /* ACK */, __ /* BEL */, __ /* BS */, __ /* HT */, __ /* LF */, __ /* VT */, __ /* FF */, __ /* CR */, __ /* SO */, __ /* SI */,
42+
__ /* DLE */, __ /* DC1 */, __ /* DC2 */, __ /* DC3 */, __ /* DC4 */, __ /* NAK */, __ /* SYN */, __ /* ETB */, __ /* CAN */, __ /* EM */, __ /* SUB */, __ /* ESC */, __ /* FS */, __ /* GS */, __ /* RS */, __ /* US */,
43+
__ /* SP */, __ /* ! */, FP /* " */, __ /* # */, __ /* $ */, __ /* % */, __ /* & */, __ /* ' */, __ /* ( */, __ /* ) */, FP /* * */, __ /* + */, __ /* , */, __ /* - */, __ /* . */, F_ /* / */,
44+
__ /* 0 */, __ /* 1 */, __ /* 2 */, __ /* 3 */, __ /* 4 */, __ /* 5 */, __ /* 6 */, __ /* 7 */, __ /* 8 */, __ /* 9 */, F_ /* : */, __ /* ; */, FP /* < */, __ /* = */, FP /* > */, FP /* ? */,
45+
__ /* @ */, __ /* A */, __ /* B */, __ /* C */, __ /* D */, __ /* E */, __ /* F */, __ /* G */, __ /* H */, __ /* I */, __ /* J */, __ /* K */, __ /* L */, __ /* M */, __ /* N */, __ /* O */,
46+
__ /* P */, __ /* Q */, __ /* R */, __ /* S */, __ /* T */, __ /* U */, __ /* V */, __ /* W */, __ /* X */, __ /* Y */, __ /* Z */, __ /* [ */, F_ /* \ */, __ /* ] */, __ /* ^ */, __ /* _ */,
47+
__ /* ` */, __ /* a */, __ /* b */, __ /* c */, __ /* d */, __ /* e */, __ /* f */, __ /* g */, __ /* h */, __ /* i */, __ /* j */, __ /* k */, __ /* l */, __ /* m */, __ /* n */, __ /* o */,
48+
__ /* p */, __ /* q */, __ /* r */, __ /* s */, __ /* t */, __ /* u */, __ /* v */, __ /* w */, __ /* x */, __ /* y */, __ /* z */, __ /* { */, FP /* | */, __ /* } */, __ /* ~ */, __ /* DEL */,
49+
// clang-format on
50+
} };
51+
}
52+
53+
_TIL_INLINEPREFIX std::wstring clean_filename(std::wstring str) noexcept
54+
{
55+
using namespace til::details;
56+
str.erase(std::remove_if(std::begin(str), std::end(str), [](auto ch) {
57+
// This lookup is branchless: It always checks the filter, but throws
58+
// away the result if ch >= 128. This is faster than using `&&` (branchy).
59+
return ((til::at(details::pathFilter, ch & 127) & F_) != 0) & (ch < 128);
60+
}),
61+
str.end());
62+
return str;
63+
}
64+
65+
_TIL_INLINEPREFIX std::wstring clean_path(std::wstring str) noexcept
66+
{
67+
using namespace til::details;
68+
str.erase(std::remove_if(std::begin(str), std::end(str), [](auto ch) {
69+
return ((til::at(details::pathFilter, ch & 127) & _P) != 0) & (ch < 128);
70+
}),
71+
str.end());
72+
return str;
73+
}
74+
75+
// is_legal_path rules on whether a path contains any non-path characters.
76+
// it **DOES NOT** rule on whether a path exists.
77+
_TIL_INLINEPREFIX constexpr bool is_legal_path(const std::wstring_view str) noexcept
78+
{
79+
using namespace til::details;
80+
return !std::any_of(std::begin(str), std::end(str), [](auto&& ch) {
81+
return ((til::at(details::pathFilter, ch & 127) & _P) != 0) & (ch < 128);
82+
});
83+
}
84+
3385
// std::string_view::starts_with support for C++17.
3486
template<typename T, typename Traits>
3587
constexpr bool starts_with(const std::basic_string_view<T, Traits>& str, const std::basic_string_view<T, Traits>& prefix) noexcept

src/til/ut_til/string.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,10 @@ class StringTests
169169
VERIFY_ARE_EQUAL("", s);
170170
}
171171
}
172+
173+
TEST_METHOD(LegalPath)
174+
{
175+
VERIFY_IS_TRUE(til::is_legal_path(LR"(C:\Users\Documents and Settings\Users\;\Why not)"));
176+
VERIFY_IS_FALSE(til::is_legal_path(LR"(C:\Users\Documents and Settings\"Quote-un-quote users")"));
177+
}
172178
};

0 commit comments

Comments
 (0)