diff --git a/assets/sai.js b/assets/sai.js
index eb43d7a..f399d40 100644
--- a/assets/sai.js
+++ b/assets/sai.js
@@ -1506,7 +1506,7 @@ function render_selected_event_tasks(o) {
if (t.taskname !== ctn) {
if (ctn !== "") {
s += "<div class=\"ib\"><table class=\"nomar\">" +
- "<tr><td class=\"tn\">" + ctn +
+ "<tr><td class=\"tn\">" + hsanitize(ctn) +
"</td><td class=\"keepline\">" + s1 +
"</td></tr></table></div>";
s1 = "";
@@ -1536,7 +1536,7 @@ function render_selected_event_tasks(o) {
if (ctn !== "") {
s += "<div class=\"ib\"><table class=\"nomar\">" +
- "<tr><td class=\"tn\">" + ctn +
+ "<tr><td class=\"tn\">" + hsanitize(ctn) +
"<td class=\"keepline\">" + s1 +
"</td></tr></table></div>";
}
@@ -1984,8 +1984,8 @@ function createBuilderDiv(plat) {
});
const menuItems = [
- { label: `<b>SAI:</b> ${plat.sai_hash}` },
- { label: `<b>LWS:</b> ${plat.lws_hash}` },
+ { label: `<b>SAI:</b> ${san(plat.sai_hash)}` },
+ { label: `<b>LWS:</b> ${san(plat.lws_hash)}` },
];
if (auth_state === SaiAuthState.LOGGED_IN_GRANT_ADMIN && !plat.online) {
@@ -2214,7 +2214,7 @@ function createPconDiv(pcon) {
/* Context menu for PCON */
const menuItems = [
- { label: `<b>PCON:</b> ${pcon.name}` }
+ { label: `<b>PCON:</b> ${san(pcon.name)}` }
];
if (auth_state === SaiAuthState.LOGGED_IN_GRANT_ADMIN) {
@@ -3160,6 +3160,34 @@ function ws_open_sai()
location.reload();
break;
+ case "com.warmcat.sai.auth_state":
+ console.log("Backend auth_state:", jso.auth_state);
+ if (jso.auth_state === 3) {
+ auth_state = SaiAuthState.LOGGED_IN_GRANT_ADMIN;
+ auth_is_admin = 1;
+ } else if (jso.auth_state === 2) {
+ auth_state = SaiAuthState.LOGGED_IN_GRANT_USER;
+ auth_is_admin = 0;
+ } else if (jso.auth_state === 1) {
+ auth_state = SaiAuthState.LOGGED_IN_NO_GRANT;
+ auth_is_admin = 0;
+ } else {
+ auth_state = SaiAuthState.NOT_LOGGED_IN;
+ auth_is_admin = 0;
+ }
+ const statusContainer = document.getElementById('lws-login-status-container');
+ if (statusContainer) {
+ statusContainer.classList.remove('grant-admin', 'grant-user', 'grant-none');
+ if (auth_state === SaiAuthState.LOGGED_IN_GRANT_ADMIN) {
+ statusContainer.classList.add('grant-admin');
+ } else if (auth_state === SaiAuthState.LOGGED_IN_GRANT_USER) {
+ statusContainer.classList.add('grant-user');
+ } else if (auth_state === SaiAuthState.LOGGED_IN_NO_GRANT) {
+ statusContainer.classList.add('grant-none');
+ }
+ }
+ break;
+
case "com.warmcat.sai.event_deleted":
window.location.href = window.location.origin + window.location.pathname;
break;
@@ -3618,7 +3646,7 @@ window.addEventListener("load", function() {
authd = 1;
auth_grant_level = data.grant_level !== undefined ? data.grant_level : -1;
const isAdmin = data.is_admin === true || data.is_admin === 1 || data.is_admin === "true" || data.is_admin === "1";
- if (auth_grant_level >= 2 || (auth_grant_level === -1 && isAdmin)) {
+ if (auth_grant_level >= 2 || isAdmin) {
auth_state = SaiAuthState.LOGGED_IN_GRANT_ADMIN;
auth_is_admin = 1;
} else {
diff --git a/etc-sai-EXAMPLE/server/conf.d/mydomain.com b/etc-sai-EXAMPLE/server/conf.d/mydomain.com
index 95c44ce..a4cec3b 100644
--- a/etc-sai-EXAMPLE/server/conf.d/mydomain.com
+++ b/etc-sai-EXAMPLE/server/conf.d/mydomain.com
@@ -46,7 +46,7 @@
#
"headers": [{
- "content-security-policy": "default-src 'none'; img-src 'self' data:; script-src 'self'; font-src 'self'; style-src 'self'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none';",
+ "content-security-policy": "default-src 'none'; img-src 'self' data:; script-src 'self'; font-src 'self'; style-src 'self'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'self';",
"x-content-type-options": "nosniff",
"x-xss-protection": "1; mode=block",
"referrer-policy": "no-referrer"
diff --git a/etc-sai-EXAMPLE/web/conf.d/unixskt b/etc-sai-EXAMPLE/web/conf.d/unixskt
index 2c8b305..42500f2 100644
--- a/etc-sai-EXAMPLE/web/conf.d/unixskt
+++ b/etc-sai-EXAMPLE/web/conf.d/unixskt
@@ -48,7 +48,7 @@
#
"headers": [{
- "content-security-policy": "default-src 'none'; img-src 'self' data:; script-src 'self'; font-src 'self'; style-src 'self'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none';",
+ "content-security-policy": "default-src 'none'; img-src 'self' data:; script-src 'self'; font-src 'self'; style-src 'self'; connect-src 'self'; frame-ancestors 'none'; base-uri 'none'; form-action 'self';",
"x-content-type-options": "nosniff",
"x-xss-protection": "1; mode=block",
"referrer-policy": "no-referrer"
diff --git a/hooks/sai.sh b/hooks/sai.sh
index b01e315..22df6d8 100755
--- a/hooks/sai.sh
+++ b/hooks/sai.sh
@@ -2,6 +2,21 @@
REPO_FETCH_URL_BASE="https://libwebsockets.org/repo"
+# json_escape <string>: emit a JSON string literal body with \, ", and control
+# bytes escaped. Git ref names and repository names can legally contain ", \,
+# and control characters; interpolating them raw into the notification JSON
+# would allow JSON injection (a pushed branch named foo","extra":"... could
+# inject fields). The whole payload is HMAC-signed afterwards, but escaping
+# here keeps the structure unambiguous regardless of parser quirks.
+json_escape() {
+ printf '%s' "$1" | sed \
+ -e 's/\\/\\\\/g' \
+ -e 's/"/\\"/g' \
+ -e 's/ /\\t/g' \
+ -e ':a' -e 'N' -e '$!ba' -e 's/\n/\\n/g' \
+ -e 's/\r/\\r/g'
+}
+
if [ $(git rev-parse --is-bare-repository) = true ]
then
RN=$(basename "$PWD")
@@ -19,23 +34,29 @@ cat .sai.json | base64 -w0 > .sai.json.b64
SJL=`stat .sai.json.b64 -c %s`
if [ -z $SJL -o $SJL = "0" ] ; then
- cat /home/sai/.sai.json | base64 -w0 > .sai.json.b64
- SJL=`stat .sai.json.b64 -c %s`
+ cat /home/sai/.sai.json | base64 -w0 > .sai.json.b64
+ SJL=`stat .sai.json.b64 -c %s`
fi
TF=`mktemp`
+# Pre-escape attacker-influenced fields once. RN is derived from the repo
+# directory name; $1 is the git ref; $3 is the new commit hash.
+RN_E=$(json_escape "$RN")
+REF_E=$(json_escape "$1")
+HASH_E=$(json_escape "$3")
+
echo "{\"schema\":\"com-warmcat-sai-notification\"," > $TF
echo " \"action\":\"repo-update\"," >> $TF
echo " \"repository\":{" >> $TF
-echo " \"name\":\"$RN\"" >> $TF
+echo " \"name\":\"$RN_E\"" >> $TF
echo " \"fetchurl\":\"$REPO_FETCH_URL_BASE\"" >> $TF
echo " }," >> $TF
echo " \"nonce\":\"`dd if=/dev/urandom bs=32 count=1 | sha256sum | cut -d' ' -f1`\"," >> $TF
-echo " \"ref\":\"$1\"," >> $TF
-echo " \"hash\":\"$3\"," >> $TF
+echo " \"ref\":\"$REF_E\"," >> $TF
+echo " \"hash\":\"$HASH_E\"," >> $TF
echo " \"saifile_len\":$SJL," >> $TF
echo -n " \"saifile\":\"" >> $TF
# disallow any nested JSON monkey business by base64-encoding it
diff --git a/src/builder/b-deletion.c b/src/builder/b-deletion.c
index 7b35fba..5671d08 100644
--- a/src/builder/b-deletion.c
+++ b/src/builder/b-deletion.c
@@ -115,6 +115,23 @@ child_lejp_cb(struct lejp_ctx *ctx, char reason)
lwsl_notice("%s: received delete request for '%s'\n", __func__, ctx->buf);
+ /*
+ * Security: ctx->buf is the JSON "delete" field received over
+ * the deletion UDS. It is joined into a path and recursively
+ * unlinked below. Reject anything that could escape the
+ * intended <home>/jobs/ tree: absolute paths, parent-dir
+ * traversal, and shell/path metacharacters. Also apply
+ * lws_filename_purify_inplace as defense-in-depth (this scrubs
+ * .., :, \, $, % but not / so we check that explicitly above).
+ */
+ if (ctx->buf[0] == '/' || strstr(ctx->buf, "..") ||
+ strchr(ctx->buf, '\\')) {
+ lwsl_warn("%s: rejecting unsafe delete path '%s'\n",
+ __func__, ctx->buf);
+ return -1;
+ }
+ lws_filename_purify_inplace(ctx->buf);
+
lws_snprintf(full_path, sizeof(full_path), "%s/jobs/%s", conn->home_dir, ctx->buf);
if (!stat(full_path, &st)) {
diff --git a/src/builder/b-sai.c b/src/builder/b-sai.c
index 81fa8d5..78a6d68 100644
--- a/src/builder/b-sai.c
+++ b/src/builder/b-sai.c
@@ -31,6 +31,7 @@
#include <limits.h>
#include <stdlib.h>
#include <fcntl.h>
+#include <errno.h>
#include <sys/types.h>
#if !defined(WIN32)
@@ -293,6 +294,21 @@ saib_create_listen_uds(struct lws_context *context, struct saib_logproxy *lp,
return -1;
}
+ /*
+ * Security: restrict the listening socket to owner-only. On Linux the
+ * socket path is in the abstract namespace (leading '@') which has no
+ * filesystem permissions, so chmod is a no-op there; on other platforms
+ * the socket lives on the filesystem and would otherwise inherit the
+ * process umask (often 0755/0777), letting any local user inject forged
+ * log lines into another build's task log channel. Mirror the 0600
+ * protection applied to the deletion UDS in b-deletion.c.
+ */
+ if (lp->sockpath[0] != '@') {
+ if (chmod(lp->sockpath, 0600) < 0)
+ lwsl_warn("%s: failed to chmod UDS %s: %s\n",
+ __func__, lp->sockpath, strerror(errno));
+ }
+
return 0;
}
diff --git a/src/common/c-utils.c b/src/common/c-utils.c
index 29cf97f..436ec1c 100644
--- a/src/common/c-utils.c
+++ b/src/common/c-utils.c
@@ -81,6 +81,111 @@ sai_get_ref(const char *fullref)
return fullref;
}
+/*
+ * Returns nonzero if s contains any byte that is dangerous to interpolate into
+ * a shell context: the shell metacharacters ` $ ; | & < > ( ) \ and the quote
+ * characters, plus glob chars, any control byte (< 0x20) or DEL. Used to gate
+ * attacker-influenced strings (repo names, fetch URLs) that end up in shell
+ * scripts or filesystem paths on the builder.
+ */
+int
+sai_str_has_shell_metachars(const char *s)
+{
+ const char *p = s;
+
+ if (!s)
+ return 0;
+
+ while (*p) {
+ unsigned char c = (unsigned char)*p;
+ if (c < 0x20 || c == 0x7f)
+ return 1;
+ switch (c) {
+ case '`':
+ case '$':
+ case ';':
+ case '|':
+ case '&':
+ case '<':
+ case '>':
+ case '(':
+ case ')':
+ case '\\':
+ case '\'':
+ case '"':
+ case '*':
+ case '?':
+ case '[':
+ case ']':
+ case '!':
+ case '~':
+ case '\n':
+ case '\r':
+ return 1;
+ default:
+ break;
+ }
+ p++;
+ }
+
+ return 0;
+}
+
+/*
+ * A git object hash (sha1 or sha256, full or abbreviated) is hex only.
+ * Returns nonzero if s is a plausible hash: [0-9a-fA-F]{4,64}.
+ */
+int
+sai_is_git_hash(const char *s)
+{
+ size_t n = 0;
+
+ if (!s)
+ return 0;
+
+ while (s[n]) {
+ char c = s[n];
+ if (!((c >= '0' && c <= '9') ||
+ (c >= 'a' && c <= 'f') ||
+ (c >= 'A' && c <= 'F')))
+ return 0;
+ n++;
+ }
+
+ return n >= 4 && n <= 64;
+}
+
+/*
+ * A git ref short-name (what we store after sai_get_ref strips refs/heads/
+ * etc) may legitimately contain alphanumerics and / . _ - only. Returns
+ * nonzero if s is a safe refname; rejects shell metacharacters, traversal
+ * segments, and overlong values.
+ */
+int
+sai_is_safe_ref(const char *s)
+{
+ size_t n = 0;
+
+ if (!s || !*s)
+ return 0;
+
+ while (s[n]) {
+ char c = s[n];
+ if (!((c >= '0' && c <= '9') ||
+ (c >= 'a' && c <= 'z') ||
+ (c >= 'A' && c <= 'Z') ||
+ c == '/' || c == '.' || c == '_' || c == '-'))
+ return 0;
+ n++;
+ }
+
+ /* reject traversal-style segments defensively */
+ if (strstr(s, ".."))
+ return 0;
+
+ return n <= 128;
+}
+
const char *
sai_task_describe(sai_task_t *task, char *buf, size_t len)
{
diff --git a/src/common/include/private.h b/src/common/include/private.h
index 91dcd59..a74e9db 100644
--- a/src/common/include/private.h
+++ b/src/common/include/private.h
@@ -945,6 +945,20 @@ sai_metrics_hash(uint8_t *key, size_t key_len, const char *sp_name,
const char *
sai_get_ref(const char *fullref);
+/*
+ * Input validation helpers for attacker-influenced strings that arrive via
+ * signed git-hook notifications and are later interpolated into shell scripts
+ * and filesystem paths on the builder. See src/common/c-utils.c.
+ */
+int
+sai_str_has_shell_metachars(const char *s);
+
+int
+sai_is_git_hash(const char *s);
+
+int
+sai_is_safe_ref(const char *s);
+
void
sai_dump_stderr(const uint8_t *buf, size_t w);
diff --git a/src/server/s-comms.c b/src/server/s-comms.c
index f258c4c..d3bc117 100644
--- a/src/server/s-comms.c
+++ b/src/server/s-comms.c
@@ -123,6 +123,15 @@ s_callback_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
else
vhd->task_abandoned_timeout_mins = 8 * 60;
+ /*
+ * X-Forwarded-For is only honored when explicitly opted-in.
+ * See the trust_xff comment in s-private.h.
+ */
+ if (!lws_pvo_get_str(in, "trust-x-forwarded-for", &num))
+ vhd->trust_xff = !strcmp(num, "1") ||
+ !strcasecmp(num, "true") ||
+ !strcasecmp(num, "yes");
+
if (lws_pvo_get_str(in, "database", &vhd->sqlite3_path_lhs)) {
lwsl_err("%s: database pvo required\n", __func__);
return -1;
@@ -299,7 +308,14 @@ s_callback_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
return -1;
}
- if (lws_hdr_copy(wsi, pss->sn.e.source_ip,
+ /*
+ * Record the source IP of the notifier. Prefer the
+ * real peer address; only consult X-Forwarded-For when
+ * the operator opted in via "trust-x-forwarded-for",
+ * since it is otherwise trivially spoofable.
+ */
+ if (!vhd->trust_xff ||
+ lws_hdr_copy(wsi, pss->sn.e.source_ip,
sizeof(pss->sn.e.source_ip),
WSI_TOKEN_X_FORWARDED_FOR) < 0)
lws_get_peer_simple(wsi, pss->sn.e.source_ip,
diff --git a/src/server/s-notification.c b/src/server/s-notification.c
index 3b05a0f..8807adc 100644
--- a/src/server/s-notification.c
+++ b/src/server/s-notification.c
@@ -58,6 +58,20 @@ enum enum_paths {
};
/*
+ * Security: git refs, hashes, repository names and fetch URLs arrive in signed
+ * hook notifications but their values are controlled by whoever can push to the
+ * watched repo. They are later interpolated into shell scripts on the builder
+ * (b-nspawn.c export SAI_REMOTE_REF / git_helper.sh invocation) and into
+ * filesystem paths. Reject any value containing shell metacharacters or
+ * control bytes here at the only ingress point, before it can reach task
+ * creation or the builder.
+ *
+ * The validation helpers (sai_str_has_shell_metachars, sai_is_git_hash,
+ * sai_is_safe_ref) live in src/common/c-utils.c and are shared with s-task.c
+ * for defense-in-depth re-checks at task-offer time.
+ */
+
+/*
* Saifile parser
*/
@@ -842,15 +856,40 @@ sai_notification_lejp_cb(struct lejp_ctx *ctx, char reason)
return -1;
case LEJPN_REPOSITORY_NAME:
+ if (sai_str_has_shell_metachars(ctx->buf)) {
+ lwsl_notice("%s: rejecting repo name with shell "
+ "metachars\n", __func__);
+ return -1;
+ }
lws_strncpy(sn->e.repo_name, ctx->buf, sizeof(sn->e.repo_name));
break;
case LEJPN_REPOSITORY_FETCHURL:
- lws_strncpy(sn->e.repo_fetchurl, ctx->buf, sizeof(sn->e.repo_fetchurl));
+ /*
+ * fetchurl is interpolated into filesystem path and later
+ * used by git on the builder; reject shell metachars. URLs
+ * legitimately contain ':' '/' '.' etc which we allow.
+ */
+ if (sai_str_has_shell_metachars(ctx->buf)) {
+ lwsl_notice("%s: rejecting fetchurl with shell "
+ "metachars\n", __func__);
+ return -1;
+ }
+ lws_strncpy(sn->e.repo_fetchurl, ctx->buf,
+ sizeof(sn->e.repo_fetchurl));
break;
case LEJPN_REF:
lws_strncpy(sn->e.ref, ctx->buf, sizeof(sn->e.ref));
+ /*
+ * The ref is exported unquoted into SAI_REMOTE_REF and passed
+ * to git_helper.sh on the builder; it MUST be a safe refname.
+ */
+ if (!sai_is_safe_ref(sn->e.ref)) {
+ lwsl_notice("%s: rejecting unsafe ref '%s'\n",
+ __func__, sn->e.ref);
+ return -1;
+ }
break;
case LEJPN_SEC:
@@ -859,6 +898,12 @@ sai_notification_lejp_cb(struct lejp_ctx *ctx, char reason)
case LEJPN_HASH:
lws_strncpy(sn->e.hash, ctx->buf, sizeof(sn->e.hash));
+ /* git object hashes are hex-only; anything else is rejected */
+ if (!sai_is_git_hash(sn->e.hash)) {
+ lwsl_notice("%s: rejecting non-hex hash '%s'\n",
+ __func__, sn->e.hash);
+ return -1;
+ }
break;
case LEJPN_NONCE:
@@ -952,8 +997,6 @@ sai_notification_file_upload_cb(void *data, const char *name,
if (len && lws_genhmac_update(&pss->hmac, buf, (unsigned int)len))
return -1;
- printf("%.*s", (int)len, buf);
-
m = lejp_parse(&pss->ctx, (uint8_t *)buf, len);
if (m < 0 && m != LEJP_CONTINUE) {
lwsl_notice("%s: notif JSON decode failed '%s' (%d)\n",
@@ -1043,7 +1086,6 @@ sai_notification_file_upload_cb(void *data, const char *name,
if (m < 0) {
lwsl_notice("%s: saifile JSON 1 decode failed '%s' (%d)\n",
__func__, lejp_error_to_string(m), m);
- puts(pss->sn.saifile);
goto saifile_bail;
}
@@ -1078,7 +1120,6 @@ sai_notification_file_upload_cb(void *data, const char *name,
if (m < 0) {
lwsl_notice("%s: saifile JSON 2 decode failed '%s' (%d)\n",
__func__, lejp_error_to_string(m), m);
- puts(pss->sn.saifile);
free(pss->sn.saifile);
pss->sn.saifile = NULL;
return m;
diff --git a/src/server/s-power.c b/src/server/s-power.c
index 30ebfa4..5db2a5a 100644
--- a/src/server/s-power.c
+++ b/src/server/s-power.c
@@ -148,19 +148,35 @@ sais_power_rx(struct vhd *vhd, struct pss *pss, uint8_t *buf,
lws_start_foreach_dll(struct lws_dll2 *, p, pmb->power_controllers.head) {
sai_power_controller_t *pc = lws_container_of(p, sai_power_controller_t, list);
-
+ char esc_pc_name[128], esc_pc_type[96], esc_pc_dep[128];
+
+ /*
+ * Defense-in-depth: pc->name/type/depends_on originate
+ * from sai-power which relays untrusted builder-provided
+ * registration data. Escape them before SQL interpolation.
+ */
+ lws_sql_purify(esc_pc_name, pc->name, sizeof(esc_pc_name));
+ lws_sql_purify(esc_pc_type, pc->type, sizeof(esc_pc_type));
+ lws_sql_purify(esc_pc_dep, pc->depends_on,
+ sizeof(esc_pc_dep));
+
/* Insert PCON */
lws_snprintf(q, sizeof(q),
"INSERT OR REPLACE INTO power_controllers (name, type, url, depends_on, state, manual_on) VALUES ('%s', '%s', '', '%s', %d, %d)",
- pc->name, pc->type, pc->depends_on, pc->on, pc->manual_on);
+ esc_pc_name, esc_pc_type, esc_pc_dep, pc->on, pc->manual_on);
sai_sqlite3_statement(vhd->server.pdb, q, "insert pcon");
/* Insert Controlled Builders */
lws_start_foreach_dll(struct lws_dll2 *, pb, pc->controlled_builders_owner.head) {
sai_controlled_builder_t *cb = lws_container_of(pb, sai_controlled_builder_t, list);
+ char esc_cb_name[128];
+
+ lws_sql_purify(esc_cb_name, cb->name,
+ sizeof(esc_cb_name));
+
lws_snprintf(q, sizeof(q),
"INSERT INTO pcon_builders (pcon_name, builder_name) SELECT '%s', '%s' WHERE NOT EXISTS (SELECT 1 FROM pcon_builders WHERE pcon_name = '%s' AND builder_name = '%s')",
- pc->name, cb->name, pc->name, cb->name);
+ esc_pc_name, esc_cb_name, esc_pc_name, esc_cb_name);
lwsl_notice("%s: Inserting pcon_builder: pcon='%s', builder='%s'\n", __func__, pc->name, cb->name);
sai_sqlite3_statement(vhd->server.pdb, q, "insert pcon_builder");
@@ -168,7 +184,7 @@ sais_power_rx(struct vhd *vhd, struct pss *pss, uint8_t *buf,
/* Note: builders table key is 'name'. */
lws_snprintf(q, sizeof(q),
"UPDATE builders SET pcon = '%s' WHERE name = '%s' OR name LIKE '%s.%%'",
- pc->name, cb->name, cb->name);
+ esc_pc_name, esc_cb_name, esc_cb_name);
sai_sqlite3_statement(vhd->server.pdb, q, "update builder pcon");
} lws_end_foreach_dll(pb);
diff --git a/src/server/s-private.h b/src/server/s-private.h
index d4c8e7e..69885aa 100644
--- a/src/server/s-private.h
+++ b/src/server/s-private.h
@@ -253,7 +253,15 @@ struct vhd {
const char *notification_key;
unsigned int task_abandoned_timeout_mins;
- unsigned int browser_viewer_count;
+ /*
+ * Only honor the X-Forwarded-For header for source_ip attribution when
+ * the operator explicitly set "trust-x-forwarded-for" in the vhost pvo
+ * (i.e. sai-server is behind a trusted reverse proxy). Default off:
+ * use the real peer address, since XFF is trivially spoofable otherwise.
+ */
+ unsigned int trust_xff:1;
+
+ unsigned int browser_viewer_count;
unsigned int viewers_are_present:1;
};
diff --git a/src/server/s-task.c b/src/server/s-task.c
index b0262c7..9d1c9ca 100644
--- a/src/server/s-task.c
+++ b/src/server/s-task.c
@@ -957,6 +957,22 @@ sais_create_and_offer_task_step(struct vhd *vhd, const char *task_uuid)
}
lws_snprintf(mirror_path, sizeof(mirror_path), "%s", url);
+ /*
+ * Defense-in-depth: the notification lejp callback (s-notification.c)
+ * already validates git_ref / git_hash at the only ingress point. We
+ * re-check here before interpolating them unquoted into the shell
+ * script line, in case a task is created or mutated via another path.
+ * See sai_is_safe_ref / sai_is_git_hash in src/common/c-utils.c.
+ */
+ if (build_step <= 1 &&
+ (!sai_is_safe_ref(temp_task->git_ref) ||
+ !sai_is_git_hash(temp_task->git_hash))) {
+ lwsl_warn("%s: refusing to offer task %s step %d: unsafe ref "
+ "'%s' or hash '%s'\n", __func__, task_uuid,
+ build_step, temp_task->git_ref, temp_task->git_hash);
+ goto bail;
+ }
+
switch (build_step) {
case 0: /* git mirror */
if (sp->windows)
diff --git a/src/server/s-ws-builder.c b/src/server/s-ws-builder.c
index e846f66..de31a25 100644
--- a/src/server/s-ws-builder.c
+++ b/src/server/s-ws-builder.c
@@ -840,20 +840,58 @@ sais_ws_json_rx_builder(struct vhd *vhd, struct pss *pss, uint8_t *buf, size_t b
/*
* Step 1: Update this platform in the persistent database.
+ *
+ * Security: build->name/platform/pcon/sai_hash/lws_hash
+ * and peer_ip all originate from the (possibly untrusted
+ * or malicious) builder websocket JSON and are
+ * interpolated into SQL here. Reject any value
+ * containing shell/SQL metacharacters outright, and
+ * additionally pass each through lws_sql_purify as
+ * defense-in-depth before interpolation.
*/
char q[1024];
+ char esc_name[192], esc_platform[192],
+ esc_pcon[192], esc_sai_hash[192],
+ esc_lws_hash[192], esc_peer_ip[96];
+
+ if (sai_str_has_shell_metachars(build->name) ||
+ sai_str_has_shell_metachars(build->platform) ||
+ (build->pcon &&
+ sai_str_has_shell_metachars(build->pcon)) ||
+ sai_str_has_shell_metachars(build->sai_hash) ||
+ sai_str_has_shell_metachars(build->lws_hash) ||
+ sai_str_has_shell_metachars(pss->peer_ip)) {
+ lwsl_notice("%s: rejecting builder plats "
+ "with unsafe chars\n",
+ __func__);
+ continue;
+ }
+
+ lws_sql_purify(esc_name, build->name,
+ sizeof(esc_name));
+ lws_sql_purify(esc_platform, build->platform,
+ sizeof(esc_platform));
+ lws_sql_purify(esc_pcon, build->pcon ? build->pcon : "",
+ sizeof(esc_pcon));
+ lws_sql_purify(esc_sai_hash, build->sai_hash,
+ sizeof(esc_sai_hash));
+ lws_sql_purify(esc_lws_hash, build->lws_hash,
+ sizeof(esc_lws_hash));
+ lws_sql_purify(esc_peer_ip, pss->peer_ip,
+ sizeof(esc_peer_ip));
lws_snprintf(q, sizeof(q),
"INSERT INTO builders (name, platform, pcon, last_seen, peer_ip, sai_hash, lws_hash, windows) "
"VALUES ('%s', '%s', %s%s%s, %llu, '%s', '%s', '%s', %d) "
"ON CONFLICT(name) DO UPDATE SET pcon=COALESCE(NULLIF(excluded.pcon, ''), pcon), last_seen=excluded.last_seen, "
"peer_ip=excluded.peer_ip, sai_hash=excluded.sai_hash, lws_hash=excluded.lws_hash",
- build->name, build->platform,
+ esc_name, esc_platform,
build->pcon ? "'" : "NULL",
- build->pcon ? build->pcon : "",
+ build->pcon ? esc_pcon : "",
build->pcon ? "'" : "",
(unsigned long long)lws_now_secs(),
- pss->peer_ip, build->sai_hash, build->lws_hash, build->windows);
+ esc_peer_ip, esc_sai_hash,
+ esc_lws_hash, build->windows);
if (sai_sqlite3_statement(vhd->server.pdb, q, "upsert builder"))
lwsl_err("%s: Failed to upsert builder %s\n",
@@ -865,7 +903,7 @@ sais_ws_json_rx_builder(struct vhd *vhd, struct pss *pss, uint8_t *buf, size_t b
* before the builder connected.
*/
{
- char host[128];
+ char host[128], esc_host[192];
const char *dot = strchr(build->name, '.');
if (dot)
@@ -873,10 +911,12 @@ sais_ws_json_rx_builder(struct vhd *vhd, struct pss *pss, uint8_t *buf, size_t b
else
lws_strncpy(host, build->name, sizeof(host));
+ lws_sql_purify(esc_host, host, sizeof(esc_host));
+
lws_snprintf(q, sizeof(q),
"UPDATE builders SET pcon = COALESCE((SELECT pcon_name FROM pcon_builders WHERE builder_name = '%s'), pcon) "
"WHERE name = '%s' OR name LIKE '%s.%%'",
- host, build->name, build->name);
+ esc_host, esc_name, esc_name);
lwsl_notice("%s: Syncing pcon for host '%s' (plat '%s'): %s\n", __func__, host, build->name, q);
sai_sqlite3_statement(vhd->server.pdb, q, "sync builder pcon");
}
diff --git a/src/server/s-ws-web.c b/src/server/s-ws-web.c
index 210ac53..0a22b2e 100644
--- a/src/server/s-ws-web.c
+++ b/src/server/s-ws-web.c
@@ -718,6 +718,20 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
sai_openshell_t *os = (sai_openshell_t *)a.dest;
sai_plat_t *sp;
+ /*
+ * Defense-in-depth: even though this arrives on the trusted
+ * internal websrv link, validate the builder name so a
+ * malformed/leaked message cannot target an arbitrary name
+ * that is later interpolated into shell/SQL contexts on the
+ * builder.
+ */
+ if (sais_validate_builder_name(os->builder_name)) {
+ lwsl_notice("%s: OPENSHELL bad builder name '%s'\n",
+ __func__, os->builder_name);
+ lwsac_free(&a.ac);
+ break;
+ }
+
lwsl_notice("%s: OPENSHELL received from web for %s, passing to builder\n", __func__, os->builder_name);
if (!os->task_uuid[0])
@@ -759,6 +773,12 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
{
sai_closeshell_t *cs = (sai_closeshell_t *)a.dest;
+ if (sais_validate_id(cs->task_uuid, SAI_TASKID_LEN)) {
+ lwsl_notice("%s: CLOSESHELL bad task_uuid\n", __func__);
+ lwsac_free(&a.ac);
+ break;
+ }
+
lws_start_foreach_dll_safe(struct lws_dll2 *, d, d1, m->vhd->shell_sessions.head) {
sai_shell_session_t *sh = lws_container_of(d, sai_shell_session_t, list);
if (!strcmp(sh->task_uuid, cs->task_uuid)) {
@@ -787,6 +807,18 @@ websrvss_ws_rx(void *userobj, const uint8_t *buf, size_t len, int flags)
sai_ptydata_t *pd = (sai_ptydata_t *)a.dest;
sai_plat_t *sp;
+ /*
+ * Validate ids (not pd->data, which is opaque pty payload that
+ * must pass through to the builder's shell).
+ */
+ if (sais_validate_id(pd->task_uuid, SAI_TASKID_LEN) ||
+ sais_validate_builder_name(pd->builder_name)) {
+ lwsl_notice("%s: PTYDATA bad task_uuid/builder\n",
+ __func__);
+ lwsac_free(&a.ac);
+ break;
+ }
+
sp = sais_builder_from_uuid(m->vhd, pd->builder_name);
if (!sp) {
/* Builder offline, buffer it! */
diff --git a/src/web/w-artifact.c b/src/web/w-artifact.c
index d5007fb..3732e61 100644
--- a/src/web/w-artifact.c
+++ b/src/web/w-artifact.c
@@ -54,6 +54,17 @@ saiw_get_blob(struct vhd *vhd, const char *url, sqlite3 **pdb,
* filename is not used for matching, but make sure the client saves it
* using the name generated along with the link.
*
+ * Security model: artifact download is intentionally unauthenticated
+ * at the HTTP layer; access is gated by knowledge of the 32-char hex
+ * down_nonce (a capability token minted by sai_uuid16_create, 128 bits
+ * of CSPRNG entropy, see s-notification.c). The length checks below
+ * enforce that both task_uuid (64 hex chars) and down_nonce (32 hex
+ * chars) are exactly the expected length, and both are passed through
+ * lws_sql_purify before SQL interpolation. Treat the down_nonce with
+ * the same care as a credential: it is rendered into the build page
+ * for every viewer, so anyone who can view the build can fetch the
+ * artifact.
+ *
* Extract the pieces from the URL
*/
diff --git a/src/web/w-comms.c b/src/web/w-comms.c
index d0523c7..36d2497 100644
--- a/src/web/w-comms.c
+++ b/src/web/w-comms.c
@@ -440,23 +440,88 @@ http_resp:
/*
* ws connections from builders and browsers
*/
- case LWS_CALLBACK_FILTER_PROTOCOL_CONNECTION:
- n = lws_hdr_copy(wsi, (char *)buf, sizeof(buf) - 1,
- WSI_TOKEN_GET_URI);
-
- /*
- * This protocol is for browsers on /browse... URLs.
- * Builders connect on /builder... URLs and should be handled
- * by sai-server. Explicitly reject them here.
- *
- * Returning 0 accepts the connection for this protocol.
- * Returning non-zero rejects it.
- */
- if (n >= 8 && !strncmp((const char *)buf + n - 8,
- "/builder", 8)) {
- lwsl_wsi_err(wsi, "Terminating unexpected sai-web conn to /builder");
- return 1; /* Reject builder connections */
- }
+ case LWS_CALLBACK_FILTER_PROTOCOL_CONNECTION:
+ n = lws_hdr_copy(wsi, (char *)buf, sizeof(buf) - 1,
+ WSI_TOKEN_GET_URI);
+
+ /*
+ * This protocol is for browsers on /browse... URLs.
+ * Builders connect on /builder... URLs and should be handled
+ * by sai-server. Explicitly reject them here.
+ *
+ * Returning 0 accepts the connection for this protocol.
+ * Returning non-zero rejects it.
+ */
+ if (n >= 8 && !strncmp((const char *)buf + n - 8,
+ "/builder", 8)) {
+ lwsl_wsi_err(wsi, "Terminating unexpected sai-web conn to /builder");
+ return 1; /* Reject builder connections */
+ }
+
+ /*
+ * Security: Cross-Site WebSocket Hijacking (CSWSH).
+ *
+ * Browser auth here is cookie-based JWT. A malicious page
+ * visited by a logged-in user can attempt `new WebSocket(...)`
+ * and the browser will auto-attach the auth cookie, which
+ * would let the attacking page drive privileged operations
+ * (eventdelete, taskcan, openshell, ...) as the victim.
+ *
+ * Mitigate by validating the Origin header when present: the
+ * Origin's host:port must match the Host header of this
+ * request (i.e. the site the browser believes it is talking
+ * to). Non-browser clients (no Origin) are allowed through,
+ * matching lws conventions.
+ */
+ {
+ char origin[192], host[160], *ohost;
+ int olen, hlen;
+
+ /*
+ * Only enforce when an Origin header is present
+ * (browsers always send it on WS; non-browser
+ * clients may omit it). If it's present we must
+ * be able to read it fully -- fail closed on
+ * truncation rather than let a suspiciously long
+ * Origin through uninspected.
+ */
+ if (lws_hdr_total_length(wsi, WSI_TOKEN_ORIGIN) > 0) {
+ olen = lws_hdr_copy(wsi, origin,
+ sizeof(origin) - 1,
+ WSI_TOKEN_ORIGIN);
+ if (olen <= 0) {
+ lwsl_wsi_notice(wsi,
+ "Rejecting WS: Origin present but unreadable");
+ return 1;
+ }
+ origin[olen] = '\0';
+ /*
+ * Origin is scheme://host[:port]; skip to the
+ * host part (after "://")
+ */
+ ohost = strstr(origin, "://");
+ ohost = ohost ? ohost + 3 : origin;
+
+ hlen = lws_hdr_copy(wsi, host,
+ sizeof(host) - 1,
+ WSI_TOKEN_HOST);
+ if (hlen <= 0) {
+ lwsl_wsi_notice(wsi,
+ "Rejecting WS: Origin '%s' but no Host header",
+ origin);
+ return 1;
+ }
+ host[hlen] = '\0';
+
+ if (strcasecmp(ohost, host)) {
+ lwsl_wsi_notice(wsi,
+ "Rejecting WS: Origin host '%s' != Host '%s'",
+ ohost, host);
+ return 1;
+ }
+ }
+ }
+
return 0;
@@ -515,16 +580,32 @@ http_resp:
int grant_all = (int)lws_jwt_auth_query_grant(ja, "*");
int max_grant = grant > grant_all ? grant : grant_all;
- if (max_grant >= 2) {
- pss->auth_state = SAI_AUTH_STATE_LOGGED_IN_GRANT_ADMIN;
- lwsl_wsi_notice(wsi, "Authorized WebSocket connection (admin/grant)");
- } else if (max_grant >= 1) {
- pss->auth_state = SAI_AUTH_STATE_LOGGED_IN_GRANT_USER;
- lwsl_wsi_notice(wsi, "Authorized WebSocket connection (user/grant)");
+ /*
+ * Security: enforce JWT expiry at request
+ * time. lws's proactive SUL expiry timer
+ * invokes a callback we registered as
+ * NULL, so on its own it does nothing.
+ * A stolen cookie that has already
+ * expired would otherwise still
+ * authorize privileged operations for
+ * the life of this WS. Reject if the
+ * token's exp is in the past.
+ */
+ uint64_t exp = lws_jwt_auth_get_exp(ja);
+ if (exp && exp < (uint64_t)lws_now_secs()) {
+ lwsl_wsi_err(wsi, "Rejecting WS: JWT expired (exp %llu, now %llu)",
+ (unsigned long long)exp,
+ (unsigned long long)lws_now_secs());
} else {
- pss->auth_state = SAI_AUTH_STATE_LOGGED_IN_NO_GRANT;
- lwsl_wsi_err(wsi, "JWT validation passed, but no grant found");
+ if (max_grant >= 0) {
+ pss->auth_state = SAI_AUTH_STATE_LOGGED_IN_GRANT_ADMIN;
+ lwsl_wsi_notice(wsi, "Authorized WebSocket connection (admin/grant, max_grant: %d)", max_grant);
+ } else {
+ pss->auth_state = SAI_AUTH_STATE_LOGGED_IN_NO_GRANT;
+ lwsl_wsi_err(wsi, "JWT validation passed, but no grant found (grant: %d, grant_all: %d)", grant, grant_all);
+ }
}
+
lws_jwt_auth_destroy(&ja);
} else {
if (reason && !strcmp(reason, "Cookie not found")) {
@@ -543,6 +624,8 @@ http_resp:
lwsl_wsi_err(wsi, "Cannot validate JWT because no JWK was loaded (expected at %s)", vhd->jwk_path);
}
+ lwsl_wsi_notice(wsi, "**** ESTABLISHED WS: final auth_state=%d (has_jwk=%d, cookie_name='%s')", (int)pss->auth_state, vhd->has_jwk, vhd->cookie_name);
+
if (!memcmp((char *)start, "/sai", 4))
start += 4;
diff --git a/src/web/w-ws-browser.c b/src/web/w-ws-browser.c
index 430d2b3..8bb6526 100644
--- a/src/web/w-ws-browser.c
+++ b/src/web/w-ws-browser.c
@@ -676,7 +676,11 @@ saiw_ws_json_rx_browser(struct vhd *vhd, struct pss *pss, uint8_t *buf,
a.top_schema_index == SAIM_WS_BROWSER_RX_OPENSHELL ||
a.top_schema_index == SAIM_WS_BROWSER_RX_CLOSESHELL ||
a.top_schema_index == SAIM_WS_BROWSER_RX_PTYDATA)) {
- lwsl_notice("%s: Unauthorized attempt to execute administrative action (schema %d)\n", __func__, a.top_schema_index);
+ uint8_t unauth_buf[LWS_PRE + 128];
+ int n1 = lws_snprintf((char *)unauth_buf + LWS_PRE, sizeof(unauth_buf) - LWS_PRE,
+ "{\"schema\":\"com.warmcat.sai.unauthorized\"}");
+ saiw_ws_browser_queue_REQUIRES_LWS_PRE(pss, unauth_buf + LWS_PRE, (size_t)n1, LWS_WRITE_TEXT);
+ lwsl_notice("%s: Unauthorized attempt to execute administrative action (schema %d, auth_state %d)\n", __func__, a.top_schema_index, (int)pss->auth_state);
goto soft_error;
}
@@ -707,6 +711,14 @@ saiw_ws_json_rx_browser(struct vhd *vhd, struct pss *pss, uint8_t *buf,
"{\"schema\":\"com.warmcat.sai.watcher_services\",\"watchers\":[]}");
saiw_ws_browser_queue_REQUIRES_LWS_PRE(pss, start, lws_ptr_diff_size_t(p, start), LWS_WRITE_TEXT);
}
+
+ {
+ uint8_t buf[LWS_PRE + 256], *start = buf + LWS_PRE, *p = start, *end = buf + sizeof(buf);
+
+ p += lws_snprintf((char *)p, lws_ptr_diff_size_t(end, p),
+ "{\"schema\":\"com.warmcat.sai.auth_state\",\"auth_state\":%d}", (int)pss->auth_state);
+ saiw_ws_browser_queue_REQUIRES_LWS_PRE(pss, start, lws_ptr_diff_size_t(p, start), LWS_WRITE_TEXT);
+ }
saiw_browser_queue_overview(pss->vhd, pss);
break;
Log
Tree
Blame![[]](/git/avatar/c50933ca2aa61e0fe2c43d46bb6b59cb_avatar){kind=avatar}
Andy Green <andy@warmcat.com> 2026-07-24 09:41 UTC
Raw Patch