format-python-brace: Limit acceptable format specifiers

4 files changed, 97 insertions, 14 deletions

diff --git a/gettext-tools/src/ChangeLog b/gettext-tools/src/ChangeLog
index 4b0f4d3..98a425f 100644
--- a/gettext-tools/src/ChangeLog
+++ b/gettext-tools/src/ChangeLog
@@ -1,3 +1,12 @@
+2014-04-30  Daiki Ueno  <ueno@gnu.org>
+
+	format-python-brace: Limit acceptable format specifiers
+	Problem reported by Kovid Goyal at:
+	<https://savannah.gnu.org/bugs/?41668>.
+	* format-python-brace.c (parse_directive): Only recognize a single
+	nested format directive or the standard format specifiers as
+	format specifiers.
+
 2014-04-22  Roumen Petrov  <bugtrack@roumenpetrov.info>  (tiny change)
 
 	build: Use Automake 'subdir-objects' option
diff --git a/gettext-tools/src/format-python-brace.c b/gettext-tools/src/format-python-brace.c
index 2081536..fa6763d 100644
--- a/gettext-tools/src/format-python-brace.c
+++ b/gettext-tools/src/format-python-brace.c
@@ -24,6 +24,7 @@
 #include <string.h>
 
 #include "format.h"
+#include "c-ctype.h"
 #include "xalloc.h"
 #include "xvasprintf.h"
 #include "format-invalid.h"
@@ -38,11 +39,11 @@
      - an identifier [_A-Za-z][_0-9A-Za-z]*|[0-9]+,
      - an optional getattr ('.') or getitem ('['..']') operator with
        an identifier as argument,
-     - an optional width specifier starting with ':', with a
+     - an optional format specifier starting with ':', with a
        (unnested) format string as argument,
      - a closing brace '}'.
    Brace characters '{' and '}' can be escaped by doubles '{{' and '}}'.
- */
+*/
 
 struct named_arg
 {
@@ -186,20 +187,82 @@ parse_directive (struct spec *spec,
           return false;
         }
 
+      /* Format specifiers.  Although a format specifier can be any
+         string in theory, we can only recognize two types of format
+         specifiers below, because otherwise we would need to evaluate
+         Python expressions by ourselves:
+
+           - A nested format directive expanding to the whole string
+           - The Standard Format Specifiers, as described in PEP3101,
+             not including a nested format directive  */
       format++;
-      if (!parse_upto (spec, &format, false, '}', translated, fdi,
-                       invalid_reason))
+      if (*format == '{')
         {
-          /* FDI and INVALID_REASON will be set by a recursive call of
-             parse_directive.  */
-          return false;
-        }
+          /* Nested format directive.  */
+          if (!parse_directive (spec, &format, false, translated, fdi,
+                                invalid_reason))
+            {
+              /* FDI and INVALID_REASON will be set by a recursive call of
+                 parse_directive.  */
+              *invalid_reason =
+                xasprintf (_(""), spec->directives, *format);
+              FDI_SET (format, FMTDIR_ERROR);
+              return false;
+            }
 
-      if (*format == '\0')
+          if (*format != '}')
+            {
+              *invalid_reason = INVALID_UNTERMINATED_DIRECTIVE ();
+              FDI_SET (format, FMTDIR_ERROR);
+              return false;
+            }
+        }
+      else
         {
-          *invalid_reason = INVALID_UNTERMINATED_DIRECTIVE ();
-          FDI_SET (format, FMTDIR_ERROR);
-          return false;
+          /* Standard format specifiers, in the form:
+             [[fill]align][sign][#][0][minimumwidth][.precision][type]  */
+
+          /* Look ahead two characters to skip [[fill]align].  */
+          int c1, c2;
+
+          c1 = format[0];
+          c2 = format[1];
+
+          if (c2 == '<' || c2 == '>' || c2 == '=' || c2 == '^')
+            format += 2;
+          else if (c1 == '<' || c1 == '>' || c1 == '=' || c1 == '^')
+            format++;
+          if (*format == '+' || *format == '-' || *format == ' ')
+            format++;
+          if (*format == '#')
+            format++;
+          if (*format == '0')
+            format++;
+          while (c_isdigit (*format))
+            format++;
+          if (*format == '.')
+            {
+              format++;
+              while (c_isdigit (*format))
+                format++;
+            }
+          switch (*format)
+            {
+            case 'b': case 'c': case 'd': case 'o': case 'x': case 'X':
+            case 'n':
+            case 'e': case 'E': case 'f': case 'F': case 'g': case 'G':
+            case '%':
+              format++;
+              break;
+            default:
+              break;
+            }
+          if (*format != '}')
+            {
+              *invalid_reason = INVALID_UNTERMINATED_DIRECTIVE ();
+              FDI_SET (format, FMTDIR_ERROR);
+              return false;
+            }
         }
       c = *format;
     }
diff --git a/gettext-tools/tests/ChangeLog b/gettext-tools/tests/ChangeLog
index cde1e77..a82a570 100644
--- a/gettext-tools/tests/ChangeLog
+++ b/gettext-tools/tests/ChangeLog
@@ -1,3 +1,8 @@
+2014-04-30  Daiki Ueno  <ueno@gnu.org>
+
+	format-python-brace: Limit acceptable format specifiers
+	* format-python-brace-1: Add tests for standard format specifiers.
+
 2014-04-29  Daiki Ueno  <ueno@gnu.org>
 
 	tests: Don't run a test only successful after "make dist" by default
diff --git a/gettext-tools/tests/format-python-brace-1 b/gettext-tools/tests/format-python-brace-1
index dba2dac..18ff861 100755
--- a/gettext-tools/tests/format-python-brace-1
+++ b/gettext-tools/tests/format-python-brace-1
@@ -24,10 +24,16 @@ cat <<\EOF > f-pyb-1.data
 "abc{value[name}"
 # Valid: format specifier
 "abc{value:0}"
+# Valid: standard format specifier
+"abc{value:<<-#012.34e}"
+# Invalid: non-standard format specifier
+"abc{value:<c>}"
 # Valid: nested format specifier
-"abc{value:0{foo}0}"
+"abc{value:{foo}}"
 # Invalid: too many nesting of format specifier
-"abc{value:0{foo:0}0}"
+"abc{value:{foo:0}}"
+# Invalid: nested format specifier, in the middle of other format specifiers
+"abc{value:0{foo}0}"
 EOF
 
 : ${XGETTEXT=xgettext}