Don't allow null chars in various strings

Various places that now reject null chars inside strings
- Primitive file operations reject it in filenames.
- Primitive environment variable operations reject it in
  names and values.
- os:cmd() reject it in its input.

Also '=' characters are rejected by primitive environment
variable operations in environment variable names.

Documentation has been updated to document null characters
in these types of data as invalid. Currently these operations
accept null chars at the end of strings, but that will change
in the future.

5 files changed, 310 insertions, 86 deletions

diff --git a/erts/emulator/beam/erl_alloc.types b/erts/emulator/beam/erl_alloc.types
index 50a1d97dd5..f8c089247e 100644
--- a/erts/emulator/beam/erl_alloc.types
+++ b/erts/emulator/beam/erl_alloc.types
@@ -285,6 +285,7 @@ type    MREF_ENT	STANDARD	SYSTEM		magic_ref_entry
 type	MREF_TAB_BKTS	STANDARD	SYSTEM		magic_ref_table_buckets
 type	MREF_TAB	LONG_LIVED	SYSTEM		magic_ref_table
 type	MINDIRECTION	FIXED_SIZE	SYSTEM		magic_indirection
+type  	OPEN_PORT_ENV 	TEMPORARY	SYSTEM		open_port_env
 
 +if threads_no_smp
 # Need thread safe allocs, but std_alloc and fix_alloc are not;
diff --git a/erts/emulator/beam/erl_bif_os.c b/erts/emulator/beam/erl_bif_os.c
index 5cd172c26f..910325a2f4 100644
--- a/erts/emulator/beam/erl_bif_os.c
+++ b/erts/emulator/beam/erl_bif_os.c
@@ -37,6 +37,8 @@
 #include "dist.h"
 #include "erl_version.h"
 
+static int check_env_name(char *name);
+
 /*
  * Return the pid for the Erlang process in the host OS.
  */
@@ -101,8 +103,10 @@ BIF_RETTYPE os_getenv_1(BIF_ALIST_1)
     key_str = erts_convert_filename_to_native(BIF_ARG_1,buf,STATIC_BUF_SIZE,
 					      ERTS_ALC_T_TMP,1,0,&len);
 
-    if (!key_str) {
-	BIF_ERROR(p, BADARG);
+    if (!check_env_name(key_str)) {
+        if (key_str && key_str != &buf[0])
+            erts_free(ERTS_ALC_T_TMP, key_str);
+        BIF_ERROR(p, BADARG);
     }
 
     if (key_str != &buf[0])
@@ -143,25 +147,20 @@ BIF_RETTYPE os_putenv_2(BIF_ALIST_2)
 {
     char def_buf_key[STATIC_BUF_SIZE];
     char def_buf_value[STATIC_BUF_SIZE];
-    char *key_buf, *value_buf;
+    char *key_buf = NULL, *value_buf = NULL;
 
     key_buf = erts_convert_filename_to_native(BIF_ARG_1,def_buf_key,
 					      STATIC_BUF_SIZE,
 					      ERTS_ALC_T_TMP,0,0,NULL);
-    if (!key_buf) {
-	BIF_ERROR(BIF_P, BADARG);
-    }
+    if (!check_env_name(key_buf))
+        goto badarg;
+
     value_buf = erts_convert_filename_to_native(BIF_ARG_2,def_buf_value,
 						STATIC_BUF_SIZE,
 						ERTS_ALC_T_TMP,1,0,
 						NULL);
-    if (!value_buf) {
-	if (key_buf != def_buf_key) {
-	    erts_free(ERTS_ALC_T_TMP, key_buf);
-	}
-	BIF_ERROR(BIF_P, BADARG);
-    }
-	    
+    if (!value_buf)
+        goto badarg;
 
     if (erts_sys_putenv(key_buf, value_buf)) {
 	if (key_buf != def_buf_key) {
@@ -179,6 +178,13 @@ BIF_RETTYPE os_putenv_2(BIF_ALIST_2)
 	erts_free(ERTS_ALC_T_TMP, value_buf);
     }
     BIF_RET(am_true);
+
+badarg:
+    if (key_buf && key_buf != def_buf_key)
+	erts_free(ERTS_ALC_T_TMP, key_buf);
+    if (value_buf && value_buf != def_buf_value)
+	erts_free(ERTS_ALC_T_TMP, value_buf);
+    BIF_ERROR(BIF_P, BADARG);
 }
 
 BIF_RETTYPE os_unsetenv_1(BIF_ALIST_1)
@@ -188,20 +194,21 @@ BIF_RETTYPE os_unsetenv_1(BIF_ALIST_1)
 
     key_buf = erts_convert_filename_to_native(BIF_ARG_1,buf,STATIC_BUF_SIZE,
 					      ERTS_ALC_T_TMP,0,0,NULL);
-    if (!key_buf) {
-	BIF_ERROR(BIF_P, BADARG);
-    }
+    if (!check_env_name(key_buf))
+        goto badarg;
+
+    if (erts_sys_unsetenv(key_buf))
+        goto badarg;
 
-    if (erts_sys_unsetenv(key_buf)) {
-	if (key_buf != buf) {
-	    erts_free(ERTS_ALC_T_TMP, key_buf);
-	}
-	BIF_ERROR(BIF_P, BADARG);
-    }
     if (key_buf != buf) {
 	erts_free(ERTS_ALC_T_TMP, key_buf);
     }
     BIF_RET(am_true);
+
+badarg:
+    if (key_buf && key_buf != buf)
+        erts_free(ERTS_ALC_T_TMP, key_buf);
+    BIF_ERROR(BIF_P, BADARG);    
 }
 
 BIF_RETTYPE os_set_signal_2(BIF_ALIST_2) {
@@ -217,3 +224,27 @@ BIF_RETTYPE os_set_signal_2(BIF_ALIST_2) {
 error:
     BIF_ERROR(BIF_P, BADARG);
 }
+
+static int
+check_env_name(char *raw_name)
+{
+    byte *c = (byte *) raw_name;
+    int encoding;
+
+    if (!c)
+        return 0;
+    
+    encoding = erts_get_native_filename_encoding();
+
+    if (erts_raw_env_char_is_7bit_ascii_char('\0', c, encoding))
+        return 0; /* Do not allow empty name... */
+
+    /* Verify no '=' characters in variable name... */
+    do {
+        if (erts_raw_env_char_is_7bit_ascii_char('=', c, encoding))
+            return 0;
+        c = erts_raw_env_next_char(c, encoding);
+    } while (!erts_raw_env_char_is_7bit_ascii_char('\0', c, encoding));
+
+    return 1; /* Seems ok... */
+}
diff --git a/erts/emulator/beam/erl_bif_port.c b/erts/emulator/beam/erl_bif_port.c
index ff03151619..ad8be8053f 100644
--- a/erts/emulator/beam/erl_bif_port.c
+++ b/erts/emulator/beam/erl_bif_port.c
@@ -45,7 +45,7 @@
 #include "dtrace-wrapper.h"
 
 static Port *open_port(Process* p, Eterm name, Eterm settings, int *err_typep, int *err_nump);
-static byte* convert_environment(Process* p, Eterm env);
+static char* convert_environment(Eterm env);
 static char **convert_args(Eterm);
 static void free_args(char **);
 
@@ -726,11 +726,11 @@ open_port(Process* p, Eterm name, Eterm settings, int *err_typep, int *err_nump)
 			goto badarg;
 		    }
 		} else if (option == am_env) {
-		    byte* bytes;
-		    if ((bytes = convert_environment(p, *tp)) == NULL) {
+                    if (opts.envir) /* ignore previous env option... */
+                        erts_free(ERTS_ALC_T_OPEN_PORT_ENV, opts.envir);
+                    opts.envir = convert_environment(*tp);
+                    if (!opts.envir)
 			goto badarg;
-		    }
-		    opts.envir = (char *) bytes;
 		} else if (option == am_args) {
 		    char **av;
 		    char **oav = opts.argv;
@@ -964,6 +964,8 @@ open_port(Process* p, Eterm name, Eterm settings, int *err_typep, int *err_nump)
 	erts_atomic32_read_bor_relb(&port->state, sflgs);
  
  do_return:
+    if (opts.envir)
+        erts_free(ERTS_ALC_T_OPEN_PORT_ENV, opts.envir);
     if (name_buf)
 	erts_free(ERTS_ALC_T_TMP, (void *) name_buf);
     if (opts.argv) {
@@ -1029,74 +1031,129 @@ static void free_args(char **av)
     }
     erts_free(ERTS_ALC_T_TMP, av);
 }
-    
 
-static byte* convert_environment(Process* p, Eterm env)
+#ifdef DEBUG
+#define ERTS_CONV_ENV_BUF_EXTRA 2
+#else
+#define ERTS_CONV_ENV_BUF_EXTRA 1024
+#endif
+
+static char* convert_environment(Eterm env)
 {
-    Eterm all;
-    Eterm* temp_heap;
-    Eterm* hp;
-    Uint heap_size;
-    Sint n;
-    Sint size;
+    /*
+     * Returns environment buffer in memory allocated
+     * as ERTS_ALC_T_OPEN_PORT_ENV. Caller *needs*
+     * to deallocate...
+     */
+
+    Sint size, alloc_size;
     byte* bytes;
     int encoding = erts_get_native_filename_encoding();
 
-    if ((n = erts_list_length(env)) < 0) {
-	return NULL;
-    }
-    heap_size = 2*(5*n+1);
-    temp_heap = hp = (Eterm *) erts_alloc(ERTS_ALC_T_TMP, heap_size*sizeof(Eterm));
-    bytes = NULL;		/* Indicating error */
+    alloc_size = ERTS_CONV_ENV_BUF_EXTRA;
+    bytes = erts_alloc(ERTS_ALC_T_OPEN_PORT_ENV,
+                       alloc_size);
+    size = 0;
 
-    /*
-     * All errors below are handled by jumping to 'done', to ensure that the memory
-     * gets deallocated. Do NOT return directly from this function.
-     */
+    /* ERTS_CONV_ENV_BUF_EXTRA >= for end delimiter... */
+    ERTS_CT_ASSERT(ERTS_CONV_ENV_BUF_EXTRA >= 2);
 
-    all = CONS(hp, make_small(0), NIL);
-    hp += 2;
+    while (is_list(env)) {
+        Sint var_sz, val_sz, need;
+        byte *str, *limit;
+	Eterm tmp, *tp, *consp;
 
-    while(is_list(env)) {
-	Eterm tmp;
-	Eterm* tp;
+        consp = list_val(env);
+	tmp = CAR(consp);
+	if (is_not_tuple_arity(tmp, 2))
+	    goto error;
 
-	tmp = CAR(list_val(env));
-	if (is_not_tuple_arity(tmp, 2)) {
-	    goto done;
-	}
 	tp = tuple_val(tmp);
-	tmp = CONS(hp, make_small(0), NIL);
-	hp += 2;
-	if (tp[2] != am_false) {
-	    tmp = CONS(hp, tp[2], tmp);
-	    hp += 2;
-	}
-	tmp = CONS(hp, make_small('='), tmp);
-	hp += 2;
-	tmp = CONS(hp, tp[1], tmp);
-	hp += 2;
-	all = CONS(hp, tmp, all);
-	hp += 2;
-	env = CDR(list_val(env));
-    }
-    if (is_not_nil(env)) {
-	goto done;
-    }
 
-    if ((size = erts_native_filename_need(all,encoding)) < 0) {
-	goto done;
+        /* Check encoding of env variable... */
+        if (is_not_list(tp[1]))
+            goto error;
+        var_sz = erts_native_filename_need(tp[1], encoding);
+        if (var_sz <= 0)
+            goto error;
+        /* Check encoding of value... */
+        if (tp[2] == am_false || is_nil(tp[2]))
+            val_sz = 0;
+        else if (is_not_list(tp[2]))
+            goto error;
+        else {
+            val_sz = erts_native_filename_need(tp[2], encoding);
+            if (val_sz < 0)
+                goto error;
+        }
+
+        /* Ensure enough memory... */
+        need = size;
+        need += var_sz + val_sz;
+        /* '=' and '\0' */
+        need += 2 * erts_raw_env_7bit_ascii_char_need(encoding);
+        if (need > alloc_size) {
+            alloc_size = (need - alloc_size) + alloc_size;
+            alloc_size += ERTS_CONV_ENV_BUF_EXTRA;
+            bytes = erts_realloc(ERTS_ALC_T_OPEN_PORT_ENV,
+                                 bytes, alloc_size);
+        }
+
+        /* Write environment variable name... */
+        str = bytes + size;
+        erts_native_filename_put(tp[1], encoding, str);
+        /* empty variable name is not allowed... */
+        if (erts_raw_env_char_is_7bit_ascii_char('\0', str, encoding))
+            goto error;
+
+        /*
+         * Drop null characters at the end and verify that we do
+         * not have any '=' characters in the name...
+         */
+        limit = str + var_sz;
+        while (str < limit) {
+            if (erts_raw_env_char_is_7bit_ascii_char('\0', str, encoding))
+                break;
+            if (erts_raw_env_char_is_7bit_ascii_char('=', str, encoding))
+                goto error;
+            str = erts_raw_env_next_char(str, encoding);
+        }
+
+        /* Write the equals sign... */
+        str = erts_raw_env_7bit_ascii_char_put('=', str, encoding);
+
+        /* Write the value... */
+        if (val_sz > 0) {
+            limit = str + val_sz;
+            erts_native_filename_put(tp[2], encoding, str);
+            while (str < limit) {
+                if (erts_raw_env_char_is_7bit_ascii_char('\0', str, encoding))
+                    break;
+                str = erts_raw_env_next_char(str, encoding);
+            }
+        }
+
+        /* Delimit... */
+        str = erts_raw_env_7bit_ascii_char_put('\0', str, encoding);
+
+        size = str - bytes;
+        ASSERT(size <= alloc_size);
+
+        env = CDR(consp);
     }
 
-    /*
-     * Put the result in a binary (no risk for a memory leak that way).
-     */
-    (void) erts_new_heap_binary(p, NULL, size, &bytes);
-    erts_native_filename_put(all,encoding,bytes);
+    /* End delimit... */
+    (void) erts_raw_env_7bit_ascii_char_put('\0', &bytes[size], encoding);
+
+    if (is_nil(env))
+        return (char *) bytes;
+
+error:
+
+    if (bytes)
+        erts_free(ERTS_ALC_T_OPEN_PORT_ENV, bytes);
 
- done:
-    erts_free(ERTS_ALC_T_TMP, temp_heap);
-    return bytes;
+    return (char *) NULL; /* error... */
 }
 
 /* ------------ decode_packet() and friends: */
diff --git a/erts/emulator/beam/erl_unicode.c b/erts/emulator/beam/erl_unicode.c
index 2d1d1443a7..b7a5c45fea 100644
--- a/erts/emulator/beam/erl_unicode.c
+++ b/erts/emulator/beam/erl_unicode.c
@@ -1988,7 +1988,7 @@ char *erts_convert_filename_to_encoding(Eterm name, char *statbuf, size_t statbu
 	is_list(name) || 
 	(allow_empty && is_nil(name))) {
 	Sint need;
-	if ((need = erts_native_filename_need(name,encoding)) < 0) {
+	if ((need = erts_native_filename_need(name, encoding)) < 0) {
 	    return NULL;
 	}
 	if (encoding == ERL_FILENAME_WIN_WCHAR) {
@@ -2152,12 +2152,13 @@ Eterm erts_convert_native_to_filename(Process *p, byte *bytes)
 }
 
 
-Sint erts_native_filename_need(Eterm ioterm, int encoding) 
+Sint erts_native_filename_need(Eterm ioterm, int encoding)
 {
     Eterm *objp;
     Eterm obj;
     DECLARE_ESTACK(stack);
     Sint need = 0;
+    int seen_null = 0;
 
     if (is_atom(ioterm)) {
 	Atom* ap;
@@ -2194,6 +2195,22 @@ Sint erts_native_filename_need(Eterm ioterm, int encoding)
 	default:
 	    need = -1;
 	}
+        /*
+         * Do not allow null in
+         * the middle of filenames
+         */
+        if (need > 0) {
+            byte *name = ap->name;
+            int len = ap->len;
+            for (i = 0; i < len; i++) {
+                if (name[i] == 0)
+                    seen_null = 1;
+                else if (seen_null) {
+                    need = -1;
+                    break;
+                }
+            }
+        }
 	DESTROY_ESTACK(stack);
 	return need;
     }
@@ -2224,6 +2241,16 @@ L_Again:   /* Restart with sublist, old listend was pushed on stack */
 		if (is_small(obj)) { /* Always small */
 		    for(;;) {
 			Uint x = unsigned_val(obj);
+                        /*
+                         * Do not allow null in
+                         * the middle of filenames
+                         */
+                        if (x == 0)
+                            seen_null = 1;
+                        else if (seen_null) {
+                            DESTROY_ESTACK(stack);
+                            return ((Sint) -1);
+                        }
 			switch (encoding) {
 			case ERL_FILENAME_LATIN1:
 			    if (x > 255) {
@@ -2497,6 +2524,38 @@ void erts_copy_utf8_to_utf16_little(byte *target, byte *bytes, int num_chars)
 }
 
 /*
+ * *** Requirements on Raw Filename Format ***
+ *
+ * These requirements are due to the 'filename' module
+ * in stdlib. This since it is documented that it
+ * should be able to operate on raw filenames as well
+ * as ordinary filenames.
+ *
+ * A raw filename *must* be a byte sequence where:
+ * 1. Codepoints 0-127 (7-bit ascii) *must* be encoded
+ *    as a byte with the corresponding value. That is,
+ *    the most significant bit in the byte encoding the
+ *    codepoint is never set.
+ * 2. Codepoints greater than 127 *must* be encoded
+ *    with the most significant bit set in *every* byte
+ *    encoding it.
+ *
+ * Latin1 and UTF-8 meet these requirements while
+ * UTF-16 and UTF-32 don't.
+ *
+ * On Windows filenames are natively stored as malformed
+ * UTF-16LE (lonely surrogates may appear). A more correct
+ * description than UTF-16 would be an array of 16-bit
+ * words... In order to meet the requirements of the
+ * raw file format we convert the malformed UTF-16LE to
+ * malformed UTF-8 which meet the requirements.
+ *
+ * Note that these requirements are today only OTP
+ * internal (erts-stdlib internal) requirements that
+ * could be changed.
+ */
+
+/*
  * This internal bif converts a filename to whatever format is suitable for the file driver
  * It also adds zero termination so that prim_file needn't bother with the character encoding
  * of the file driver 
@@ -2507,6 +2566,12 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1)
     Sint need;
     Eterm bin_term;
     byte* bin_p;
+
+    /*
+     * See comment on "Requirements on Raw Filename Format"
+     * above.
+     */
+
     /* Prim file explicitly does not allow atoms, although we could 
        very well cope with it. Instead of letting 'file' handle them,
        it would probably be more efficient to handle them here. Subject to 
@@ -2515,6 +2580,7 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1)
 	BIF_ERROR(BIF_P,BADARG);
     }
     if (is_binary(BIF_ARG_1)) {
+        int seen_null = 0;
 	byte *temp_alloc = NULL;
 	byte *bytes;
 	byte *err_pos;
@@ -2524,10 +2590,18 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1)
 	size = binary_size(BIF_ARG_1);
 	bytes = erts_get_aligned_binary_bytes(BIF_ARG_1, &temp_alloc);
 	if (encoding != ERL_FILENAME_WIN_WCHAR) {
+            Uint i;
 	    /*Add 0 termination only*/
 	    bin_term = new_binary(BIF_P, NULL, size+1);
 	    bin_p = binary_bytes(bin_term);
-	    memcpy(bin_p,bytes,size);
+            for (i = 0; i < size; i++) {
+                /* Don't allow null in the middle of filenames... */
+                if (bytes[i] == 0)
+                    seen_null = 1;
+                else if (seen_null)
+                    goto bin_name_error;
+                bin_p[i] = bytes[i];
+            }
 	    bin_p[size]=0;
 	    erts_free_aligned_binary_bytes(temp_alloc);
 	    BIF_RET(bin_term);
@@ -2541,6 +2615,11 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1)
 	    bin_term = new_binary(BIF_P, 0, (size+1)*2);
 	    bin_p = binary_bytes(bin_term);
 	    while (size--) {
+                /* Don't allow null in the middle of filenames... */
+                if (*bytes == 0)
+                    seen_null = 1;
+                else if (seen_null)
+                    goto bin_name_error;
 		*bin_p++ = *bytes++;
 		*bin_p++ = 0;
 	    }
@@ -2558,11 +2637,14 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1)
 	bin_p[num_chars*2+1] = 0;
 	erts_free_aligned_binary_bytes(temp_alloc);
 	BIF_RET(bin_term);
+    bin_name_error:
+        erts_free_aligned_binary_bytes(temp_alloc);
+        BIF_ERROR(BIF_P,BADARG);
     } /* binary */   
 	    
 
-    if ((need = erts_native_filename_need(BIF_ARG_1,encoding)) < 0) {
-	BIF_ERROR(BIF_P,BADARG);
+    if ((need = erts_native_filename_need(BIF_ARG_1, encoding)) < 0) {
+        BIF_ERROR(BIF_P,BADARG);
     }
     if (encoding == ERL_FILENAME_WIN_WCHAR) {
 	need += 2;
@@ -2596,6 +2678,11 @@ BIF_RETTYPE prim_file_internal_native2name_1(BIF_ALIST_1)
     Eterm ret;
     int mac = 0;
 
+    /*
+     * See comment on "Requirements on Raw Filename Format"
+     * above.
+     */
+
     if (is_not_binary(BIF_ARG_1)) {
 	BIF_ERROR(BIF_P,BADARG);
     }
diff --git a/erts/emulator/beam/sys.h b/erts/emulator/beam/sys.h
index 704d567337..ce751acdd3 100644
--- a/erts/emulator/beam/sys.h
+++ b/erts/emulator/beam/sys.h
@@ -1341,4 +1341,52 @@ int erts_get_printable_characters(void);
 
 void erts_init_sys_common_misc(void);
 
+ERTS_GLB_INLINE Sint erts_raw_env_7bit_ascii_char_need(int encoding);
+ERTS_GLB_INLINE byte *erts_raw_env_7bit_ascii_char_put(byte c, byte *p,
+                                                       int encoding);
+ERTS_GLB_INLINE int  erts_raw_env_char_is_7bit_ascii_char(byte c, byte *p,
+                                                          int encoding);
+ERTS_GLB_INLINE byte *erts_raw_env_next_char(byte *p, int encoding);
+
+#if ERTS_GLB_INLINE_INCL_FUNC_DEF
+
+ERTS_GLB_INLINE Sint
+erts_raw_env_7bit_ascii_char_need(int encoding)
+{
+    return (encoding == ERL_FILENAME_WIN_WCHAR) ? 2 : 1;
+}
+
+ERTS_GLB_INLINE byte *
+erts_raw_env_7bit_ascii_char_put(byte c,
+                                 byte *p,
+                                 int encoding)
+{
+    *(p++) = c;
+    if (encoding == ERL_FILENAME_WIN_WCHAR)
+        *(p++) = 0;
+    return p;
+}
+
+ERTS_GLB_INLINE int
+erts_raw_env_char_is_7bit_ascii_char(byte c,
+                                     byte *p,
+                                     int encoding)
+{
+    if (encoding == ERL_FILENAME_WIN_WCHAR)
+        return (p[0] == c) & (p[1] == 0);
+    else
+        return p[0] == c;
+}
+
+ERTS_GLB_INLINE byte *
+erts_raw_env_next_char(byte *p, int encoding)
+{
+    if (encoding == ERL_FILENAME_WIN_WCHAR)
+        return p + 2;
+    else
+        return p + 1;
+}
+
+#endif /* #if ERTS_GLB_INLINE_INCL_FUNC_DEF */
+
 #endif