948d0399a8d2e1f294fb04c82c8d2fbec735707a
max
  Tue Aug 11 07:33:13 2026 -0700
edw: fix heap overflow (edwWebXSendFile) and submit command injection (edwLib)

edwWebXSendFile tokenValid()/getFullFileName() used strcat onto exact-sized
cloneString buffers, overrunning the heap; tokenValid() does this before the
auth token is checked, so it is reachable without valid credentials. Build the
strings in right-sized buffers instead.

edwAddSubmitJob() interpolated a user-supplied submission URL into a shell
command later run by edwRunDaemon; a single quote broke out of the single-quote
context and injected commands. Reject quotes/newlines in url and userEmail and
single-quote both values in the command.

refs #38055, refs #38060

diff --git src/hg/encode3/encodeDataWarehouse/lib/edwLib.c src/hg/encode3/encodeDataWarehouse/lib/edwLib.c
index 78d5139618d..5ea3983c47e 100644
--- src/hg/encode3/encodeDataWarehouse/lib/edwLib.c
+++ src/hg/encode3/encodeDataWarehouse/lib/edwLib.c
@@ -905,33 +905,42 @@
     submit->id);
 return sqlQuickNum(conn, query);
 }
 
 boolean edwSubmitIsValidated(struct edwSubmit *submit, struct sqlConnection *conn)
 /* Return TRUE if validation has run.  This does not mean that they all passed validation.
  * It just means the validator has run and has made a decision on each file in the submission. */
 {
 /* Is this off by one because of the validated.txt being in the submission but never validated? */
 return edwSubmitCountErrors(submit,conn) + edwSubmitCountNewValid(submit, conn) == submit->newFiles;
 }
 
 void edwAddSubmitJob(struct sqlConnection *conn, char *userEmail, char *url, boolean update)
 /* Add submission job to table and wake up daemon. */
 {
+/* The command built below is later run through a shell by edwRunDaemon, with url and
+ * userEmail each wrapped in single quotes.  Inside single quotes every character is literal
+ * except a single quote itself, so a quote (or a newline) in either value would break out of
+ * the quoting and inject arbitrary shell commands.  Refuse those characters. refs #38060 */
+if (strpbrk(url, "'\n\r") != NULL)
+    errAbort("Illegal character in submission URL.");
+if (strpbrk(userEmail, "'\n\r") != NULL)
+    errAbort("Illegal character in submission user.");
+
 /* Create command and add it to edwSubmitJob table. */
 char command[strlen(url) + strlen(userEmail) + 256];
-safef(command, sizeof(command), "edwSubmit %s'%s' %s", (update ? "-update " : ""), url, userEmail);
+safef(command, sizeof(command), "edwSubmit %s'%s' '%s'", (update ? "-update " : ""), url, userEmail);
 char query[strlen(command)+128];
 sqlSafef(query, sizeof(query), "insert edwSubmitJob (commandLine) values('%s')", command);
 sqlUpdate(conn, query);
 
 /* Write sync signal (any string ending with newline) to fifo to wake up daemon. */
 FILE *fifo = mustOpen("../userdata/edwSubmit.fifo", "w");
 fputc('\n', fifo);
 carefulClose(&fifo);
 }
 
 
 struct edwValidFile *edwFindElderReplicates(struct sqlConnection *conn, struct edwValidFile *vf)
 /* Find all replicates of same output and format type for experiment that are elder
  * (fileId less than your file Id).  Younger replicates are responsible for taking care 
  * of correlations with older ones.  Sorry younguns, it's like social security. */