From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 4 Jul 2022 18:59:25 +0000 (-0400)
Subject: always escape 'octets' in fr_value_box_print()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=62c4c5797a7040bde2eb6029423c7bd93d0db2b7;p=thirdparty%2Ffreeradius-server.git

always escape 'octets' in fr_value_box_print()

fr_value_box_list_aprint() calls fr_value_box_print() to print
normal types.  And fr_value_box_print() prints 'octets' as hex
base16.

However, fr_value_box_list_aprint() calls
fr_value_box_list_concat_as_string() for 'group' types, and
that function just copies 'octets' to the output string.  Which
results in non-grouped 'octets' being printed as hex, and grouped
'octets' bring printed as binary.

the solution is to update fr_value_box_print() to enforce hex/octets
printing of 'octets'

At the same time, update the escaping logic so that if the octets
string is tainted, OR there are escaping rules, we just escape
the raw octets value.  The previous code printed it as hex, and
then escaped that, which doesn't make much sense.
---

diff --git a/src/lib/util/value.c b/src/lib/util/value.c
index e8aeee90998..008d7425305 100644
--- a/src/lib/util/value.c
+++ b/src/lib/util/value.c
@@ -5215,6 +5215,20 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff
 		break;
 
 	case FR_TYPE_GROUP:
+	{
+		fr_sbuff_escape_rules_t my_e_rules;
+
+		/*
+		 *	If the caller didn't ask to escape binary data
+		 *	in 'octets' types, then we force that now.
+		 *	Otherwise any 'octets' type which is buried
+		 *	inside of a 'group' will get copied verbatim
+		 *	from input to output, with no escaping!
+		 */
+		if (!e_rules || (!e_rules->do_oct && !e_rules->do_hex)) {
+			e_rules = &fr_value_escape_double;
+		}
+
 		/*
 		 *	Represent groups as:
 		 *
@@ -5226,6 +5240,7 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff
 				", ", (sizeof(", ") - 1), e_rules,
 				0, false);
 		FR_SBUFF_IN_CHAR_RETURN(&our_out, '}');
+	}
 		break;
 
 	case FR_TYPE_NULL:
@@ -5318,9 +5333,14 @@ ssize_t fr_value_box_list_concat_as_string(bool *tainted, fr_sbuff_t *sbuff, fr_
 			break;
 
 		case FR_TYPE_OCTETS:
-			if (vb->tainted && e_rules) goto cast;
-
-			slen = fr_sbuff_in_bstrncpy(&our_sbuff, (char const *)vb->vb_strvalue, vb->vb_length);
+			/*
+			 *	Copy the raw string over, if necessary with escaping.
+			 */
+			if (e_rules && (vb->tainted || e_rules->do_oct || e_rules->do_hex)) {
+				slen = fr_sbuff_in_escape(&our_sbuff, (char const *)vb->vb_strvalue, vb->vb_length, e_rules);
+			} else {
+				slen = fr_sbuff_in_bstrncpy(&our_sbuff, (char const *)vb->vb_strvalue, vb->vb_length);
+			}
 			break;
 
 		case FR_TYPE_STRING:
diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt
index 1539fa6d4b6..97b20c00157 100644
--- a/src/tests/unit/xlat/cond_base.txt
+++ b/src/tests/unit/xlat/cond_base.txt
@@ -264,14 +264,15 @@ match ERROR offset 0: Failed parsing string as type 'uint32'
 xlat_purify <ipaddr>127.0.0.1 == "127.0.0.1"
 match true
 
-# Invalid cast from octets to ipaddr.
+# LHS is IPaddr, RHS is string.
 xlat_purify <ipaddr>127.0.0.1 == "%{md4: 127.0.0.1}"
-match NULL
+match false
 
 #
 #  Bare %{...} is allowed.
 #
 
+# Invalid cast from octets to ipaddr.
 xlat_purify <ipaddr>127.0.0.1 == %{md4:127.0.0.1}
 match NULL
 
@@ -357,7 +358,11 @@ xlat_purify (&User-Name == (string) %{md4: blah})
 match (&User-Name == 0x544924d05ec4481925ba3749a096a0a7)
 
 xlat_purify (&User-Name == "%{md4: blah}")
-match (&User-Name == "TI$\320^\304H\031\%\2727I\240\226\240\247")
+match (&User-Name == "0x544924d05ec4481925ba3749a096a0a7")
+
+#  and without the double quotes.
+xlat_purify (&User-Name == %{md4: blah})
+match (&User-Name == 0x544924d05ec4481925ba3749a096a0a7)
 
 xlat_purify <ipaddr>127.0.0.1 == 2130706433
 match true
@@ -756,4 +761,4 @@ match %{rcode:'handled'}
 
 
 count
-match 312
+match 314