From 7d968f9dc79f346c8a95914ff4bf839d0b2fe91a Mon Sep 17 00:00:00 2001 From: Devon Kirk Date: Tue, 30 Jun 2026 19:45:03 -0400 Subject: [PATCH] fix: reject percent-encoded showStudypad/showImage in D-Bus reference The substring denylist in ipc_object_set_current_reference() runs against the raw reference bytes before main_url_handler parses the URL. libsword's URL::getParameterValue percent-decodes the action parameter, so a reference like 'passagestudy.jsp?action=show%53tudypad' (or show%49mage for showImage) contains no literal 'showStudypad'/'showImage' substring, slips past the denylist, and dispatches the local-file action against the Xiphos user. Replace the substring denylist with a percent-decoding of the action parameter and a direct string compare against the blocked actions. The substring allowlist (must contain sword://, bible://, passagestudy.jsp, or xiphos.url) is kept as a fast pre-filter. --- poc/reproducer.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ src/gtk/ipc.c | 49 +++++++++++++++++++++++++-- 2 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 poc/reproducer.c diff --git a/poc/reproducer.c b/poc/reproducer.c new file mode 100644 index 000000000..212d04c84 --- /dev/null +++ b/poc/reproducer.c @@ -0,0 +1,88 @@ +/* + * Reproduce the DBus denylist bypass: + * 1) The raw reference does NOT contain the literal "showStudypad" or "showImage". + * 2) But after libsword URL parsing, the action parameter is decoded to "showStudypad". + * 3) The handler dispatches to show_studypad / show_image. + */ +#include +#include + +/* mimic ipc.c:224-234 */ +static int ipc_check(const char *reference) { + if (!reference) return 0; + int allow = (strstr(reference, "sword://") || + strstr(reference, "bible://") || + strstr(reference, "passagestudy.jsp") || + strstr(reference, "xiphos.url")); + int deny = (strstr(reference, "showStudypad") || + strstr(reference, "showImage")); + return allow && !deny; +} + +/* mimic url.cc:886-914 re-encoding step (only /, :, space) */ +static void re_encode(const char *url, char *out) { + int j = 0; + for (int i = 0; url[i]; i++) { + char c = url[i]; + if (c == '/') { out[j++]='%'; out[j++]='2'; out[j++]='F'; } + else if (c == ':') { out[j++]='%'; out[j++]='3'; out[j++]='A'; } + else if (c == ' ') { out[j++]='%'; out[j++]='2'; out[j++]='0'; } + else out[j++] = c; + } + out[j] = 0; +} + +/* mimic libsword URL::decode (from sword-1.9.0/src/utilfuns/url.cpp:247) */ +static int hexval(char c) { + if (c>='0'&&c<='9') return c-'0'; + if (c>='A'&&c<='F') return c-'A'+10; + if (c>='a'&&c<='f') return c-'a'+10; + return -1; +} +static void url_decode(const char *in, char *out) { + int j=0; + for (int i=0; in[i]; i++) { + if (in[i]=='+') { out[j++]=' '; continue; } + if (in[i]=='%' && in[i+1] && in[i+2]) { + int a=hexval(in[i+1]), b=hexval(in[i+2]); + if (a>=0 && b>=0) { out[j++]=(char)(a*16+b); i+=2; continue; } + } + out[j++]=in[i]; + } + out[j]=0; +} + +int main() { + const char *tests[] = { + "passagestudy.jsp?action=showStudypad&value=evil.spt", /* direct: blocked */ + "passagestudy.jsp?action=show%53tudypad&value=evil.spt", /* %53 = S: bypass */ + "passagestudy.jsp?action=showImage&value=/tmp/x.png", /* direct: blocked */ + "passagestudy.jsp?action=show%49mage&value=/tmp/x.png", /* %49 = I: bypass */ + NULL + }; + for (int i=0; tests[i]; i++) { + const char *ref = tests[i]; + printf("=== ref: %s\n", ref); + printf(" ipc_check: %s\n", ipc_check(ref) ? "ACCEPTED" : "REJECTED"); + char renc[1024]; + re_encode(ref, renc); + printf(" re-encoded: %s\n", renc); + /* find ?action= */ + const char *q = strstr(renc, "action="); + if (q) { + q += 7; + char raw_val[256] = {0}; + int j=0; + for (; q[j] && q[j] != '&' && q[j] != '#'; j++) raw_val[j]=q[j]; + raw_val[j]=0; + char decoded[256]; + url_decode(raw_val, decoded); + printf(" action (raw): %s\n", raw_val); + printf(" action (decoded):%s\n", decoded); + printf(" matches 'showStudypad'? %s\n", !strcmp(decoded,"showStudypad")?"YES":"no"); + printf(" matches 'showImage'? %s\n", !strcmp(decoded,"showImage")?"YES":"no"); + } + printf("\n"); + } + return 0; +} diff --git a/src/gtk/ipc.c b/src/gtk/ipc.c index 13217d20c..1b307c4f4 100644 --- a/src/gtk/ipc.c +++ b/src/gtk/ipc.c @@ -33,6 +33,7 @@ #include #include #include +#include #include "gui/ipc.h" #include "marshal.h" #include "ipc-gdbus.h" @@ -220,18 +221,60 @@ gboolean ipc_object_set_current_reference(IpcObject *obj, * Restrict remote callers to navigation references only: the URL must * use one of the known navigation schemes/markers and must not request * a local-file action. Reject everything else (fail closed). + * + * The action check has to look at the decoded action, not the raw + * reference bytes: a reference like + * "passagestudy.jsp?action=show%53tudypad" contains no literal + * "showStudypad" substring, but libsword's URL::getParameterValue + * percent-decodes the action before main_url_handler dispatches it, + * so a substring denylist on the raw reference is bypassable. */ if (!reference || (!g_strstr_len(reference, -1, "sword://") && !g_strstr_len(reference, -1, "bible://") && !g_strstr_len(reference, -1, "passagestudy.jsp") && - !g_strstr_len(reference, -1, "xiphos.url")) || - g_strstr_len(reference, -1, "showStudypad") || - g_strstr_len(reference, -1, "showImage")) { + !g_strstr_len(reference, -1, "xiphos.url"))) { g_warning("ipc: rejected non-navigation reference: %s", reference ? reference : "(null)"); return FALSE; } + { + const char *p = strstr(reference, "action="); + gchar *action = NULL; + if (p) { + p += 7; /* strlen("action=") */ + const char *end = p; + while (*end && *end != '&' && *end != '#') + end++; + gsize len = end - p; + action = g_malloc(len + 1); + gsize i = 0, j = 0; + while (i < len) { + if (p[i] == '+') { + action[j++] = ' '; + i++; + } else if (p[i] == '%' && i + 2 < len && + g_ascii_isxdigit(p[i+1]) && + g_ascii_isxdigit(p[i+2])) { + gchar hex[3] = { p[i+1], p[i+2], 0 }; + action[j++] = (gchar)strtoul(hex, NULL, 16); + i += 3; + } else { + action[j++] = p[i++]; + } + } + action[j] = '\0'; + } + if (action && *action && + (!strcmp(action, "showStudypad") || + !strcmp(action, "showImage"))) { + g_warning("ipc: rejected local-file action: %s", + action); + g_free(action); + return FALSE; + } + g_free(action); + } main_url_handler((const gchar *)reference, TRUE);