[m-rev.] for review: avoid "poison null" security vulnerabilities

[Simon Taylor]

On 14-Mar-2007, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
> On Mon, 12 Mar 2007, Simon Taylor wrote:
> >library/string.m:
> >	Throw an exception if null characters are found in
> >	string.from_char_list and string.from_rev_char_list.
> >
> >	Add string.from_char_list_semidet and 
> >	string.from_rev_char_list_semidet
> >	which fail rather throwing an exception.  This doesn't match the
> >	normal naming convention, but string.from_{,rev_}char_list are widely
> >	used, so changing their determinism would be a bit too disruptive.
> 
> I suggest calling them string.semidet_from_char_list and 
> string.semidet_from_rev_char_list which is closer to the normal naming
> convention.  (I think the last time this came up we just ending breaking
> backwards comptability and sticking with the "correct" naming convention
> but in this case I agree, it would be too disruptive.)

Done.
 
> >library/io.m:
> >	Make io.read_line_as_string and io.read_file_as_string fail
> >	if the input file contains a null character.
> 
> `fail' is not a particularly good word to use in this context. 

Fixed.

> >Index: library/char.m
> >===================================================================
> >RCS file: /home/mercury1/repository/mercury/library/char.m,v
> >retrieving revision 1.56
> >diff -u -u -r1.56 char.m
> >--- library/char.m	13 Feb 2007 01:58:52 -0000	1.56
> >+++ library/char.m	9 Mar 2007 08:36:00 -0000
> >@@ -43,6 +43,11 @@
> >    % represent characters using Unicode, but store files in an 8-bit 
> >    national
> >    % character set.
> >    %
> >+    % Note that '\0' is not accepted as a Mercury character constant.
> 
> Add:
> 
> 	for the null character.
> 
> to the end of that sentence.

Done.


diff -u NEWS NEWS
--- NEWS
+++ NEWS
@@ -18,11 +18,12 @@
 Changes to the Mercury standard library:
 
 * Predicates and functions which create strings from lists of characters
-  now throw an exception if a null character is found.  Unexpected null
-  characters in strings are a potential source of security vulnerabilities.
+  now fail, throw an exception or return an error value if a null character
+  is found.  Unexpected null characters in strings are a potential source of
+  security vulnerabilities.
 
-  Predicates string.from_char_list_semidet/2 and
-  string.from_rev_char_list_semidet/2 have been added.  These fail rather
+  Predicates string.semidet_from_char_list/2 and
+  string.semidet_from_rev_char_list/2 have been added.  These fail rather
   than throwing an exception if a null character is found.
 
 * string.float_to_string now trims redundant trailing zeroes (although
diff -u library/char.m library/char.m
--- library/char.m
+++ library/char.m
@@ -43,7 +43,7 @@
     % represent characters using Unicode, but store files in an 8-bit national
     % character set.
     %
-    % Note that '\0' is not accepted as a Mercury character constant.
+    % Note that '\0' is not accepted as a Mercury null character constant.
     % Instead, a null character can be created using `char.det_from_int(0)'.
     % Null characters aren't very useful in Mercury because they aren't
     % allowed in strings.
diff -u library/lexer.m library/lexer.m
--- library/lexer.m
+++ library/lexer.m
@@ -2316,7 +2316,7 @@
 :- pred rev_char_list_to_string(list(char)::in, string::out) is semidet.
 
 rev_char_list_to_string(RevChars, String) :-
-   string.from_rev_char_list_semidet(RevChars, String).
+   string.semidet_from_rev_char_list(RevChars, String).
 
 :- func null_character_error = token.
 
diff -u library/string.m library/string.m
--- library/string.m
+++ library/string.m
@@ -270,7 +270,7 @@
 
     % As above, but fail instead of throwing an exception if the
     % list contains a null character.
-:- pred string.from_char_list_semidet(list(char)::in, string::uo) is semidet.
+:- pred string.semidet_from_char_list(list(char)::in, string::uo) is semidet.
 
     % Same as string.from_char_list, except that it reverses the order
     % of the characters.
@@ -281,7 +281,7 @@
 
     % As above, but fail instead of throwing an exception if the
     % list contains a null character.
-:- pred string.from_rev_char_list_semidet(list(char)::in, string::uo)
+:- pred string.semidet_from_rev_char_list(list(char)::in, string::uo)
     is semidet.
 
     % Converts a signed base 10 string to an int; throws an exception
@@ -1273,14 +1273,14 @@
 string.from_char_list(Chars::out, Str::in) :-
     string.to_char_list(Str, Chars).
 string.from_char_list(Chars::in, Str::uo) :-
-    ( string.from_char_list_semidet(Chars, Str0) ->
+    ( string.semidet_from_char_list(Chars, Str0) ->
         Str = Str0
     ;
         error("string.from_char_list: null character in list")
     ).
 
 :- pragma foreign_proc("C",
-    string.from_char_list_semidet(CharList::in, Str::uo),
+    string.semidet_from_char_list(CharList::in, Str::uo),
     [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "{
@@ -1328,16 +1328,16 @@
     Str[size] = '\\0';
 }").
 
-:- pragma promise_equivalent_clauses(string.from_char_list_semidet/2).
+:- pragma promise_equivalent_clauses(string.semidet_from_char_list/2).
 
-string.from_char_list_semidet(CharList::in, Str::uo) :-
+string.semidet_from_char_list(CharList::in, Str::uo) :-
     (
         CharList = [],
         Str = ""
     ;
         CharList = [C | Cs],
         \+ char_to_int(C, 0),
-        string.from_char_list_semidet(Cs, Str0),
+        string.semidet_from_char_list(Cs, Str0),
         string.first_char(Str, C, Str0)
     ).
 
@@ -1348,14 +1348,14 @@
 % it improves the overall speed of parsing by about 7%.
 
 string.from_rev_char_list(Chars, Str) :-
-    ( string.from_rev_char_list_semidet(Chars, Str0) ->
+    ( string.semidet_from_rev_char_list(Chars, Str0) ->
         Str = Str0
     ;
         error("string.from_rev_char_list: null character in list")
     ).
 
 :- pragma foreign_proc("C",
-    string.from_rev_char_list_semidet(Chars::in, Str::uo),
+    string.semidet_from_rev_char_list(Chars::in, Str::uo),
     [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "{
@@ -1401,8 +1401,8 @@
     }
 }").
 
-string.from_rev_char_list_semidet(Chars::in, Str::uo) :-
-    string.from_char_list_semidet(list.reverse(Chars), Str).
+string.semidet_from_rev_char_list(Chars::in, Str::uo) :-
+    string.semidet_from_char_list(list.reverse(Chars), Str).
 
 %---------------------------------------------------------------------------%
 
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------