Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 2 Mar 2002 20:24:37 +0200
From:      Alexey Zelkin <phantom@FreeBSD.org>
To:        audit@FreeBSD.org
Cc:        ache@FreeBSD.org
Subject:   safety checking for catgets (NLS catalogs)
Message-ID:  <20020302202437.A1078@gate.sim.ionidea.com>

next in thread | raw e-mail | index | archive | help
Folks,

In order to prepare of some internationalization work
for base tree tools I have reviewed some security cases
applied to NLS catalogs handling system. 

Actually the only thing which make cause problems is
possibly stale catalogs or intentionaly broken ones.

I have wrote patch which is responsible for runtime
checking of strings retrived by NLS system from message
catalog and default (not translated) version of this string
provided by application. In case if formating specifiers
in these strings are different, misplaced or not balanced
catgets() will return default version of the message instead
of retrived. It guarantees that no unexpected effects will appear
while processing of pre-loaded NLS messages with printf()'like
functions.

Also this patch is adding some kind of runtime verbosity
information. It's not related to security, but actually
useful for developers.


Index: catgets.3
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc/nls/catgets.3,v
retrieving revision 1.11
diff -u -r1.11 catgets.3
--- catgets.3	1 Oct 2001 16:08:56 -0000	1.11
+++ catgets.3	2 Mar 2002 18:09:41 -0000
@@ -52,12 +52,47 @@
 .Fa s
 points to a default message which is returned if the function
 is unable to retrieve the specified message.
+.Sh SECURITY CONSIDERATIONS
+Before returning of message retrived from the message catalog
+.Fn catgets
+will perform internal sanity check of retrived message and default
+message provided by application. Aim of this check is to make
+sure that formating specifiers provided by both messages are
+same and balanced. If check is failed (messages have different
+format specifiers) default message is returned.
+.Sh ENVIRONMENT
+The following environment variables affect the execution of the
+.Fn catgets
+function:
+.Bl -tag -width ".Ev NLS_VERBOSE"
+.It Ev NLS_VERBOSE
+If the environment variable
+.Ev NLS_VERBOSE
+is set, catgets will report runtime inconsistences of opened
+message catalog to stdandard error output. See
+.Sx DIAGNOSTIC MESSAGES
+for more information.
+.El
 .Sh RETURN VALUES
 If the specified message was retrieved successfully,
 .Fn catgets
 returns a pointer to an internal buffer containing the message string;
 otherwise it returns
 .Fa s .
+.Sh DIAGNOSTIC MESSAGES
+If the environment variable
+.Ev NLS_VERBOSE
+is set, following diagnostics messages may appear in standard error
+output:
+.Pp
+.Bl -diag
+.It "(NLS verbose): unsafe message: setId = XX, msgId = YY"
+Internal sanity check for formatting specifiers of retrived from the
+message catalog string against default message provided by application
+is failed.
+.It "(NLS verbose): missing message: setId = XX, msgId = YY"
+Requested message is not available in the specified message catalog.
+.El
 .Sh SEE ALSO
 .Xr gencat 1 ,
 .Xr catclose 3 ,
Index: msgcat.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc/nls/msgcat.c,v
retrieving revision 1.38
diff -u -r1.38 msgcat.c
--- msgcat.c	13 Aug 2001 14:06:26 -0000	1.38
+++ msgcat.c	2 Mar 2002 18:13:25 -0000
@@ -58,15 +58,24 @@
 
 #define _DEFAULT_NLS_PATH "/usr/share/nls/%L/%N.cat:/usr/share/nls/%N/%L:/usr/local/share/nls/%L/%N.cat:/usr/local/share/nls/%N/%L"
 
+/*
+ * List of valid conversion specifiers for *printf() and strfmon(). These
+ * are used to determine safety of the message catalog's version of string.
+ */ 
+#define VALID_CONV_SPECIFIERS "cDdiAaEefFgGnOopsUuXx"
+
 #define	TRUE	1
 #define	FALSE	0
 
 #define	NLERR		((nl_catd) -1)
 #define	NLRETERR(errc)	errno = errc; return(NLERR);
 
+static int nls_verbose = FALSE;
+
 static nl_catd loadCat();
 static int loadSet();
 static void __nls_free_resources();
+static int msg_unsafe();
 
 nl_catd
 catopen( name, type)
@@ -83,6 +92,8 @@
 	NLRETERR(ENOENT);
   }
 
+  nls_verbose = (getenv("NLS_VERBOSE") == NULL) ? FALSE : TRUE;
+
   /* is it absolute path ? if yes, load immidiately */
   if (strchr(name, '/'))
 	return loadCat(name);
@@ -271,12 +282,24 @@
     if (catd == NULL || catd == NLERR)
 	return((char *)dflt);
     msg = MCGetMsg(MCGetSet(cat, setId), msgId);
-    if (msg != NULL) cptr = msg->msg.str;
-    else cptr = dflt;
+    if (msg != NULL) {
+        cptr = msg->msg.str;
+	if (msg_unsafe(cptr, dflt)) {
+	    if (nls_verbose == TRUE)
+		printf ("(NLS verbose): unsafe message: setId = %d, msgId = %d\n"
+			"(NLS verbose): original = \"%s\",\n"
+			"(NLS verbose): default = \"%s\"\n", setId, msgId, cptr, dflt);
+	    cptr = dflt;
+	}
+    } else {
+	if (nls_verbose == TRUE)
+	    printf ("(NLS verbose): missing message: setId = %d, msgId = %d\n"
+		    "(NLS verbose): default = \"%s\"\n", setId, msgId, dflt);
+	cptr = dflt;
+    }
     return((char *)cptr);
 }
 
-
 int
 catclose( catd)
 	nl_catd catd;
@@ -457,4 +480,58 @@
     }
     set->invalid = FALSE;
     return(1);
+}
+
+/*
+ * Return pointer to first valid '%'.
+ * '%%' case handled as usual character.
+ */
+static char *
+msg_skip(ptr)
+	char	*ptr;
+{
+    while (ptr) {
+	while (*ptr != '%' && *ptr != '\0') ptr++;
+	if (*ptr == '\0')
+	    break;
+	ptr++;
+	if (*ptr != '%') {
+	    ptr--;
+	    break;
+	}
+	ptr++;
+    }
+    return (ptr);
+}
+
+/*
+ * Validate message retrived from the message catalog against default
+ * message provided by the application. Main aim of this check is to make
+ * sure that format specifiers are same and balanced in both cases.
+ */ 
+static int
+msg_unsafe(src, dflt)
+	char	*src;
+	char	*dflt;
+{
+    if (src == NULL || dflt == NULL)
+	return (1);
+
+    src = msg_skip(src);
+    dflt = msg_skip(dflt);
+    while (*src != '\0' && *dflt != '\0') {
+	while (1) {
+	    if (*dflt != *src)
+		return (1);
+	    if (strchr(VALID_CONV_SPECIFIERS, *dflt))
+		break;
+	    src++;
+	    dflt++;
+	}
+	src = msg_skip(src);
+	dflt = msg_skip(dflt);
+    }
+    if (*src != *dflt)
+	return (1);
+    return (0);
 }

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020302202437.A1078>