pidl s4::NDR::Parser: read hex numbers as numbers for ranges
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Sat, 30 Nov 2019 10:37:08 +0000 (23:37 +1300)
committerAndrew Bartlett <abartlet@samba.org>
Tue, 10 Dec 2019 02:53:35 +0000 (02:53 +0000)
Hex numbers in IDL are not parsed as numbers, resulting in warnings
like

Argument 0x2000 isn't numeric in numeric lt (<) at /home/douglas/src/samba/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm line 981

not to mention problematic code. We add a utility function to convert
these numbers to numbers.

A typical difference this makes is:

 --- old/default/librpc/gen_ndr/ndr_dcerpc.c     2019-11-30 23:40:32.915816967 +1300
 +++ new/default/librpc/gen_ndr/ndr_dcerpc.c     2019-11-30 17:00:09.055733660 +1300
 @@ -1893,7 +1893,7 @@
         if (ndr_flags & NDR_SCALARS) {
                 NDR_CHECK(ndr_pull_align(ndr, 4));
                 NDR_CHECK(ndr_pull_uint32(ndr, NDR_SCALARS, &r->ReceiveWindowSize));
 -               if (r->ReceiveWindowSize > 0x40000) {
 +               if (r->ReceiveWindowSize < 8192 || r->ReceiveWindowSize > 262144) {
                         return ndr_pull_error(ndr, NDR_ERR_RANGE, "value out of range");
                 }
                 NDR_CHECK(ndr_pull_trailer_align(ndr, 4));

Where the minimum ("0x2000" == 8192) was read as a string, thus
treated as zero.

The treatment as zero was introduced in 142b2a61f8a77b3065ce4c78b459ab714d6d190a
accidentially, which shows why warnings are important.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
pidl/lib/Parse/Pidl/Util.pm

index ae85f2a46bd9b9979cde160a78c7b6d376980873..276588351a2fa5202ac395e97fe4855c584827ab 100644 (file)
@@ -15,7 +15,13 @@ push @ISA, qw(Exporter);
 use strict;
 use warnings;
 use Parse::Pidl::Typelist qw(hasType getType mapTypeName typeHasBody);
-use Parse::Pidl::Util qw(has_property ParseExpr ParseExprExt print_uuid unmake_str);
+use Parse::Pidl::Util qw(has_property
+                        ParseExpr
+                        ParseExprExt
+                        print_uuid
+                        unmake_str
+                        parse_int
+                        parse_range);
 use Parse::Pidl::CUtil qw(get_pointer_to get_value_of get_array_element);
 use Parse::Pidl::NDR qw(GetPrevLevel GetNextLevel ContainsDeferred ContainsPipe is_charset_array);
 use Parse::Pidl::Samba4 qw(is_intree choose_header ArrayDynamicallyAllocated);
@@ -337,7 +343,7 @@ sub ParseArrayPullGetSize($$$$$$)
        my $array_size = "size_$e->{NAME}_$l->{LEVEL_INDEX}";
 
        if (my $range = has_property($e, "range")) {
-               my ($low, $high) = split(/,/, $range, 2);
+               my ($low, $high) = parse_range($range);
                if ($low < 0) {
                        warning(0, "$low is invalid for the range of an array size");
                }
@@ -372,7 +378,7 @@ sub ParseArrayPullGetLength($$$$$$;$)
        my $array_length = "length_$e->{NAME}_$l->{LEVEL_INDEX}";
 
        if (my $range = has_property($e, "range")) {
-               my ($low, $high) = split(/,/, $range, 2);
+               my ($low, $high) = parse_range($range);
                if ($low < 0) {
                        warning(0, "$low is invalid for the range of an array size");
                }
@@ -977,7 +983,7 @@ sub ParseDataPull($$$$$$$)
                if ($range and $pl->{TYPE} ne "ARRAY") {
                        $var_name = get_value_of($var_name);
                        my $signed = Parse::Pidl::Typelist::is_signed($l->{DATA_TYPE});
-                       my ($low, $high) = split(/,/, $range, 2);
+                       my ($low, $high) = parse_range($range);
                        if ($low < 0 and not $signed) {
                                warning(0, "$low is invalid for the range of an unsigned type");
                        }
index 2bcd4e4476146f540ae347bbc79b573106b357c6..7a6039ba12b37b63a93c2052776efed51a452379 100644 (file)
@@ -6,7 +6,7 @@ package Parse::Pidl::Util;
 
 require Exporter;
 @ISA = qw(Exporter);
-@EXPORT = qw(has_property property_matches ParseExpr ParseExprExt is_constant make_str unmake_str print_uuid MyDumper genpad);
+@EXPORT = qw(has_property property_matches ParseExpr ParseExprExt is_constant make_str unmake_str print_uuid MyDumper genpad parse_int parse_range);
 use vars qw($VERSION);
 $VERSION = '0.01';
 
@@ -191,6 +191,41 @@ sub genpad($)
        return "\t"x($nt)." "x($ns);
 }
 
+=item B<parse_int>
+
+Try to convert hex and octal strings to numbers. If a string doesn't
+look hexish or octish it will be left as is. If the unconverted string
+is actually a decimal number, Perl is likely to handle it correctly.
+
+=cut
+
+sub parse_int {
+       my $s = shift;
+       if ($s =~ /^0[xX][0-9A-Fa-f]+$/) {
+               return hex $s;
+       }
+       if ($s =~ /^0[0-7]+$/) {
+               return oct $s;
+       }
+       return $s;
+}
+
+=item B<parse_range>
+
+Read a range specification that might contain hex or octal numbers,
+and work out what those numbers are.
+
+=cut
+
+sub parse_range {
+       my $range = shift;
+       my ($low, $high) = split(/,/, $range, 2);
+       $low = parse_int($low);
+       $high = parse_int($high);
+       return ($low, $high);
+}
+
+
 =back
 
 =cut