Created
March 31, 2026 07:16
-
-
Save jquast/4abdbbadb353ff3f6ed6ebd95d12fbf9 to your computer and use it in GitHub Desktop.
Fix command injection and screen scraping in SyncTERM HEAD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| diff --git a/src/conio/CMakeLists.txt b/src/conio/CMakeLists.txt | |
| index 274f4ef1a..c0336ab54 100644 | |
| --- a/src/conio/CMakeLists.txt | |
| +++ b/src/conio/CMakeLists.txt | |
| @@ -10,6 +10,7 @@ set(WITHOUT_X11 OFF CACHE BOOL "Disable X11 video support") | |
| set(WITHOUT_XRANDR OFF CACHE BOOL "Disable X11 video support") | |
| set(WITHOUT_XRENDER OFF CACHE BOOL "Disable X11 video support") | |
| set(WITHOUT_XINERAMA OFF CACHE BOOL "Disable X11 video support") | |
| +set(WITHOUT_DECRQCRA ON CACHE BOOL "Disable DECRQCRA rectangular area checksum (screen scraping prevention)") | |
| INCLUDE (CheckSymbolExists) | |
| find_package(Threads REQUIRED) | |
| @@ -251,6 +252,10 @@ endif() | |
| if(HAVE_VASPRINTF) | |
| target_compile_definitions(ciolib PRIVATE HAVE_VASPRINTF) | |
| endif() | |
| + | |
| +if(WITHOUT_DECRQCRA) | |
| + target_compile_definitions(ciolib PRIVATE WITHOUT_DECRQCRA) | |
| +endif() | |
| target_link_libraries(ciolib ${CMAKE_THREAD_LIBS_INIT}) | |
| target_link_libraries(ciolib hash encode xpdev) | |
| diff --git a/src/conio/cterm.c b/src/conio/cterm.c | |
| index 8bc6d8b22..77ac5bd0b 100644 | |
| --- a/src/conio/cterm.c | |
| +++ b/src/conio/cterm.c | |
| @@ -4398,6 +4398,7 @@ static void do_ansi(struct cterminal *cterm, char *retbuf, size_t retsize, int * | |
| cterm->decscs_speed = newspeed; | |
| } | |
| } | |
| +#ifndef WITHOUT_DECRQCRA | |
| else if (strcmp(seq->ctrl_func, "*y") == 0) { | |
| if (seq->param_count >= 6) { | |
| if (seq->param_int[0] != UINT64_MAX && | |
| @@ -4461,6 +4462,7 @@ static void do_ansi(struct cterminal *cterm, char *retbuf, size_t retsize, int * | |
| } | |
| } | |
| } | |
| +#endif | |
| else if (strcmp(seq->ctrl_func, "*z") == 0) { | |
| if (seq->param_count > 0 && seq->param_int[0] <= 63) { | |
| if (cterm->macros[seq->param_int[0]]) { | |
| @@ -5597,9 +5599,10 @@ static void do_ansi(struct cterminal *cterm, char *retbuf, size_t retsize, int * | |
| } | |
| break; | |
| default: | |
| - cterm_respond(cterm, "\x1bP0$r", 5); | |
| - cterm_respond(cterm, &cterm->strbuf[2], strlen(&cterm->strbuf[2])); | |
| - cterm_respond(cterm, "\x1b_", 2); | |
| + /* CVE-2008-2383 / CVE-2022-45872: never echo | |
| + * unsupported query back — fixed "not valid" | |
| + * response only, properly ST-terminated. */ | |
| + cterm_respond(cterm, "\x1bP0$r\x1b\\", 7); | |
| } | |
| } | |
| } |
Bug ticket created.
Author
Yes of course, I made a joke post on a message net about it, nobody is using syncterm to telnet/ssh to serious stuff, that someone could be creative, knowing for example their clients are using synchronet, to send a payload to cause them to post a friend request to tom
it's all in good fun, it was a bit more serious to be found left in WezTerm, thanks for taking a look.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No bother at all, always glad to hear from anyone using SyncTERM (but not with a command shell on a *nix system!).
Yeah, it's a clear oversight... SyncTERM wasn't forked from xterm, but it does tend to use XTerm as an authoritative source for extensions in preference to the VT commands. I'll fix that.
It was the DECRQCRA part that was "Oh no, nobody should be doing that!" if/when I support that use case, it will use a completely different function table, there's a lot more (like SSA/ESA/STS) that are much worse. If you want to build a version of SyncTERM suitable for use with a POSIX shell, there's a LOT more higher priority work to be done than DECRQCRA.