[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Wed Dec 18 08:06:02 UTC 2019


The branch, master has been updated
       via  ad9a81c6a93 librpc: Move winstation.idl to the top level and exclude from fuzzing
       via  5eac5813cc4 lib/fuzzing and librpc: Do not generate fuzzers for pointless targets
       via  bbc4ebbcaf3 lib/fuzzer: Allow coverage build for oss-fuzz
       via  4d9753dfbd8 lib ldb: fix use after free
       via  0e651b4508d librpc: Fix manually written printer for drsuapi_DsAttributeValue
       via  5ccb5e23c96 sefltest: Demonstrate crash in manually written printer for drsuapi_DsAttributeValue
       via  5a989d66709 lib/fuzzing: Allow load of fuzz inputs as files on the command line
       via  66d12eb98ab lib/fuzzing: Initialise st buffer in fuzz_ndr_X
      from  1141fbe9842 smbd: Convert share_mode_data->num_share_modes into a boolean8

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit ad9a81c6a933558e8ae5ccbdcb329e9effc82bb0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 15:56:55 2019 +1300

    librpc: Move winstation.idl to the top level and exclude from fuzzing
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Wed Dec 18 08:05:05 UTC 2019 on sn-devel-184

commit 5eac5813cc42ab4f17858e61cc512bace4d3bad2
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 15:34:34 2019 +1300

    lib/fuzzing and librpc: Do not generate fuzzers for pointless targets
    
    We need to focus the fuzzing effort on reachable code, and these IDL
    are just historical artifacts, many are entirely [todo] and have
    no samba client nor server.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit bbc4ebbcaf3e602cf11816a24e58f4dc3eed7d8d
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 14:48:38 2019 +1300

    lib/fuzzer: Allow coverage build for oss-fuzz
    
    This still does not seem to be enough but it is one step towards a working
    coverage build.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 4d9753dfbd8a2d66d86f60aabee627826247aa38
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Wed Dec 18 11:17:51 2019 +1300

    lib ldb: fix use after free
    
    Fix ASAN detected use after free.  No security implications as  the
    talloc_free is followed immediately by the print statement and the value
    printed is an integer
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 0e651b4508d44e4343c83e1c157d919150aee489
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 12:19:37 2019 +1300

    librpc: Fix manually written printer for drsuapi_DsAttributeValue
    
    Credit to OSS-Fuzz
    
    Found using the ndr_fuzz_X target.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 5ccb5e23c96a6555e03fac66a382780df56bcfe6
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 12:20:35 2019 +1300

    sefltest: Demonstrate crash in manually written printer for drsuapi_DsAttributeValue
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 5a989d6670911e16aed9061a8c6a74676f4c8a8e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 12:01:01 2019 +1300

    lib/fuzzing: Allow load of fuzz inputs as files on the command line
    
    This is easier to put under gdb.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 66d12eb98aba10948f829d08b4144969ead5ddbb
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 13 22:41:10 2019 +1300

    lib/fuzzing: Initialise st buffer in fuzz_ndr_X
    
    An NDR pull of a function will fill in either the in. or out.
    elements of this structure, but never both.
    
    However, some structures have size_is() in the out. that reference
    the in. elements.  This is the reason for the --context-file option
    in ndrdump.
    
    We have a special handler in the fuzzing case embedded in the
    pidl-generated output to cope with this, by filling in pointers
    for elements declared [ref,in] but it relies on the in-side
    (at least) of the buffer being zeroed.
    
    So zero the buffer before we start.  Sadly this means things
    like valgrind can not find a use of uninitialised data, but that
    is a price we have to pay.
    
    Credit to OSS-Fuzz
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

-----------------------------------------------------------------------

Summary of changes:
 lib/fuzzing/afl-fuzz-main.c                        | 14 ++++
 lib/fuzzing/fuzz_ndr_X.c                           | 10 +++
 lib/fuzzing/oss-fuzz/build_samba.sh                |  3 +
 lib/fuzzing/wscript_build                          |  3 -
 lib/ldb/common/ldb.c                               |  2 +-
 {source4/librpc => librpc}/idl/winstation.idl      |  0
 librpc/idl/wscript_build                           | 23 +++----
 librpc/ndr/ndr_drsuapi.c                           | 15 +++--
 librpc/wscript_build                               |  5 ++
 python/samba/tests/blackbox/ndrdump.py             | 12 ++++
 source4/librpc/idl/wscript_build                   |  2 +-
 .../librpc/tests/fuzzed_drsuapi_DsGetNCChanges.txt | 76 ++++++++++++++++++++++
 source4/librpc/wscript_build                       |  6 --
 13 files changed, 140 insertions(+), 31 deletions(-)
 rename {source4/librpc => librpc}/idl/winstation.idl (100%)
 create mode 100644 source4/librpc/tests/fuzzed_drsuapi_DsGetNCChanges.txt


Changeset truncated at 500 lines:

diff --git a/lib/fuzzing/afl-fuzz-main.c b/lib/fuzzing/afl-fuzz-main.c
index 730aa39ae49..9f677557867 100644
--- a/lib/fuzzing/afl-fuzz-main.c
+++ b/lib/fuzzing/afl-fuzz-main.c
@@ -28,6 +28,20 @@ int main(int argc, char *argv[]) {
 	size_t size = 0;
 #ifdef __AFL_LOOP
 	while (__AFL_LOOP(1000))
+#else
+	int i;
+	for (i = 0; i < argc; i++) {
+		uint8_t *buf = (uint8_t *)file_load(argv[i],
+						    &size,
+						    0,
+						    NULL);
+		ret = LLVMFuzzerTestOneInput(buf, size);
+		TALLOC_FREE(buf);
+		if (ret != 0) {
+			return ret;
+		}
+	}
+	if (i == 0)
 #endif
 	{
 		uint8_t *buf = (uint8_t *)fd_load(0, &size, 0, NULL);
diff --git a/lib/fuzzing/fuzz_ndr_X.c b/lib/fuzzing/fuzz_ndr_X.c
index 5fc21dcef26..e8c3bb4cf76 100644
--- a/lib/fuzzing/fuzz_ndr_X.c
+++ b/lib/fuzzing/fuzz_ndr_X.c
@@ -251,6 +251,16 @@ int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
 			TALLOC_FREE(mem_ctx);
 			return 0;
 		}
+
+		/*
+		 * We must initialise the buffer (even if we would
+		 * prefer not to for the sake of eg valgrind) as
+		 * otherwise the special handler for 'out pointer with
+		 * [size_is()] refers to in value with [ref]' fails to
+		 * trigger
+		 */
+		memset(st, '\0', sizeof(st));
+
 		ndr_pull->flags |= LIBNDR_FLAG_REF_ALLOC;
 
 		if (type == TYPE_OUT) {
diff --git a/lib/fuzzing/oss-fuzz/build_samba.sh b/lib/fuzzing/oss-fuzz/build_samba.sh
index 63b81af0810..93039e4dbe0 100755
--- a/lib/fuzzing/oss-fuzz/build_samba.sh
+++ b/lib/fuzzing/oss-fuzz/build_samba.sh
@@ -34,6 +34,9 @@ case "$SANITIZER" in
     undefined)
 	SANITIZER_ARG='--undefined-sanitizer'
 	;;
+    coverage)
+       SANITIZER_ARG=''
+       ;;
 esac
 
 # $LIB_FUZZING_ENGINE is provided by the oss-fuzz "compile" command
diff --git a/lib/fuzzing/wscript_build b/lib/fuzzing/wscript_build
index e77eea88df5..75c41ac83f4 100644
--- a/lib/fuzzing/wscript_build
+++ b/lib/fuzzing/wscript_build
@@ -97,7 +97,6 @@ bld.SAMBA_NDR_FUZZ('FileServerVssAgent') # fsvrp.idl
 bld.SAMBA_NDR_FUZZ('lsarpc') # lsa.idl
 bld.SAMBA_NDR_FUZZ('netdfs') # dfs.idl
 bld.SAMBA_NDR_FUZZ('nfs4acl_interface') # nfs4acl.idl
-bld.SAMBA_NDR_FUZZ('ObjectRpcBaseTypes') # orpc.idl
 bld.SAMBA_NDR_FUZZ('rpcecho') # echo.idl
 
 # quota.idl
@@ -113,8 +112,6 @@ bld.SAMBA_NDR_FUZZ('sparse')
 bld.SAMBA_NDR_FUZZ('resiliency')
 bld.SAMBA_NDR_FUZZ('trim')
 
-# Skipped: dsbackup (all todo)
-
 # WMI tables
 bld.SAMBA_NDR_FUZZ('IWbemClassObject')
 bld.SAMBA_NDR_FUZZ('IWbemServices')
diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 95e9138a56b..8c86dca45a1 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -1392,8 +1392,8 @@ int ldb_op_default_callback(struct ldb_request *req, struct ldb_reply *ares)
 	}
 
 	if (ares->type != LDB_REPLY_DONE) {
-		talloc_free(ares);
 		ldb_asprintf_errstring(req->handle->ldb, "Invalid LDB reply type %d", ares->type);
+		TALLOC_FREE(ares);
 		return ldb_request_done(req, LDB_ERR_OPERATIONS_ERROR);
 	}
 
diff --git a/source4/librpc/idl/winstation.idl b/librpc/idl/winstation.idl
similarity index 100%
rename from source4/librpc/idl/winstation.idl
rename to librpc/idl/winstation.idl
diff --git a/librpc/idl/wscript_build b/librpc/idl/wscript_build
index 5dda944ca71..b66f27be901 100644
--- a/librpc/idl/wscript_build
+++ b/librpc/idl/wscript_build
@@ -71,38 +71,34 @@ bld.SAMBA_PIDL_LIST('PIDL',
                     generate_fuzzers=False)
 
 # DCE/RPC protocols which Samba does not implement a client or server
-# for
+# for.  We don't generate a fuzzer for these as they are unreachable
+#
+# Do not include IDL with public structures in this list as we want to
+# fuzz those
 
 bld.SAMBA_PIDL_LIST('PIDL',
                     '''
                     audiosrv.idl
+                    dbgidl.idl
+                    dsbackup.idl
                     efs.idl
                     frstrans.idl
                     frsrpc.idl
                     keysvc.idl
                     msgsvc.idl
+                    orpc.idl
                     policyagent.idl
                     rot.idl
                     scerpc.idl
                     trkwks.idl
                     w32time.idl
+                    winstation.idl
                     wzcsvc.idl
                     ''',
                     options='--header --ndr-parser',
+                    generate_fuzzers=False,
                     output_dir='../gen_ndr')
 
-# The interface names here are not the same as the IDL name, so the
-# auto-genration of the fuzzer fails to link
-
-bld.SAMBA_PIDL_LIST('PIDL',
-                    '''
-                    dsbackup.idl
-                    orpc.idl
-                    ''',
-                    options='--header --ndr-parser',
-                    output_dir='../gen_ndr',
-                    generate_fuzzers=False)
-
 # Non-DCE/RPC protocols encoded in IDL for Samba or helper IDLs for
 # DCE/RPC protocols (eg defining constands or structures but not
 # functions)
@@ -110,7 +106,6 @@ bld.SAMBA_PIDL_LIST('PIDL',
                     '''
                     bkupblobs.idl
                     cab.idl
-                    dbgidl.idl
                     file_id.idl
                     fscc.idl
                     fsrvp_state.idl
diff --git a/librpc/ndr/ndr_drsuapi.c b/librpc/ndr/ndr_drsuapi.c
index b03c1ec57b3..cda2e2cf3c5 100644
--- a/librpc/ndr/ndr_drsuapi.c
+++ b/librpc/ndr/ndr_drsuapi.c
@@ -113,12 +113,15 @@ static void _print_drsuapi_DsAttributeValue_str(struct ndr_print *ndr, const cha
 
 	ndr_print_struct(ndr, name, "drsuapi_DsAttributeValue");
 	ndr->depth++;
-	if (!convert_string_talloc(ndr,
-	                           CH_UTF16, CH_UNIX,
-	                           r->blob->data,
-	                           r->blob->length,
-	                           &p, &converted_size)) {
-		ndr_print_string(ndr, "string", "INVALID CONVERSION");
+	if (r->blob == NULL || r->blob->data == NULL) {
+		ndr_print_string(ndr, "string", "NULL");
+	} else if (!convert_string_talloc(ndr,
+					  CH_UTF16, CH_UNIX,
+					  r->blob->data,
+					  r->blob->length,
+					  &p, &converted_size)) {
+		ndr_print_DATA_BLOB(ndr, "string (INVALID CONVERSION)",
+				    *r->blob);
 	} else {
 		char *str = (char *)p;
 		ndr_print_string(ndr, "string", str);
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 50cbed7e824..5eb78e6010a 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -449,6 +449,11 @@ bld.SAMBA_SUBSYSTEM('NDR_MESSAGING',
     public_deps='ndr NDR_SERVER_ID'
     )
 
+bld.SAMBA_SUBSYSTEM('NDR_WINSTATION',
+	source='gen_ndr/ndr_winstation.c',
+	public_deps='ndr'
+	)
+
 bld.SAMBA_SUBSYSTEM('RPC_NDR_ATSVC',
     source='gen_ndr/ndr_atsvc_c.c',
     public_deps='dcerpc-binding NDR_ATSVC'
diff --git a/python/samba/tests/blackbox/ndrdump.py b/python/samba/tests/blackbox/ndrdump.py
index 834e0fde966..ee34753e5a8 100644
--- a/python/samba/tests/blackbox/ndrdump.py
+++ b/python/samba/tests/blackbox/ndrdump.py
@@ -411,3 +411,15 @@ dump OK
             self.fail(e)
 
         self.assertEqual(actual, expected)
+
+    # Test a print of NULL pointer in manually-written ndr_drsuapi.c
+    def test_fuzzed_drsuapi_DsGetNCChanges(self):
+        expected =  open(self.data_path("fuzzed_drsuapi_DsGetNCChanges.txt"), 'rb').read()
+        try:
+            actual = self.check_output(
+                "ndrdump drsuapi 3 out --base64-input --input " +\
+                "AQAAAAEAAAAGAKoAAAAGAKoGAAMAAQAAAAYAEwAAAAAAAAAA/wAAAAAAAAA/AAAAAAAAAAAAAAAAAAAAAABbAAAAAAAAAAAAAAkRAAABAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAPkAAAAAAAABAAD4BgATAAAAAAAAAAD/AAAAAAAAAD8AAAAAAAAAAAAAAAAAAAAAAFsAAAAAAAAAAAAABgAQAAEAAAABAAAAAQAAAAEAAAABAAAAAQAAAAMAAAABAAAACREAAAEAAAABAAAAAAAAAAYAEAABAAgAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAA=")
+        except BlackboxProcessError as e:
+            self.fail(e)
+
+        self.assertEqual(actual, expected)
diff --git a/source4/librpc/idl/wscript_build b/source4/librpc/idl/wscript_build
index f8dca6af4a7..58555e6bf55 100644
--- a/source4/librpc/idl/wscript_build
+++ b/source4/librpc/idl/wscript_build
@@ -7,7 +7,7 @@ topinclude=os.path.join(bld.srcnode.abspath(), 'librpc/idl')
 bld.SAMBA_PIDL_LIST('PIDL',
 		    source='''ntp_signd.idl
                               opendb.idl sasl_helpers.idl
-                              winsif.idl winsrepl.idl winstation.idl''',
+                              winsif.idl winsrepl.idl''',
                     options="--includedir=%s --header --ndr-parser" % topinclude,
                     output_dir='../gen_ndr')
 
diff --git a/source4/librpc/tests/fuzzed_drsuapi_DsGetNCChanges.txt b/source4/librpc/tests/fuzzed_drsuapi_DsGetNCChanges.txt
new file mode 100644
index 00000000000..d688e7b40e9
--- /dev/null
+++ b/source4/librpc/tests/fuzzed_drsuapi_DsGetNCChanges.txt
@@ -0,0 +1,76 @@
+pull returned Success
+WARNING! 4 unread bytes
+[0000] 00 00 00 00                                        .... 
+    drsuapi_DsGetNCChanges: struct drsuapi_DsGetNCChanges
+        out: struct drsuapi_DsGetNCChanges
+            level_out                : *
+                level_out                : 0x00000001 (1)
+            ctr                      : *
+                ctr                      : union drsuapi_DsGetNCChangesCtr(case 1)
+                ctr1: struct drsuapi_DsGetNCChangesCtr1
+                    source_dsa_guid          : 00aa0006-0000-0006-aa06-000300010000
+                    source_dsa_invocation_id : 13000600-0000-0000-0000-0000ff000000
+                    naming_context           : NULL
+                    old_highwatermark: struct drsuapi_DsReplicaHighWaterMark
+                        tmp_highest_usn          : 0x0000000000000000 (0)
+                        reserved_usn             : 0x005b000000000000 (25614222880669696)
+                        highest_usn              : 0x0000000000000000 (0)
+                    new_highwatermark: struct drsuapi_DsReplicaHighWaterMark
+                        tmp_highest_usn          : 0x0000010000110900 (1099512744192)
+                        reserved_usn             : 0x0000000000000100 (256)
+                        highest_usn              : 0x0000000000000000 (0)
+                    uptodateness_vector      : NULL
+                    mapping_ctr: struct drsuapi_DsReplicaOIDMapping_Ctr
+                        num_mappings             : 0x00000000 (0)
+                        mappings                 : NULL
+                    extended_ret             : UNKNOWN_ENUM_VALUE (0xF900)
+                    object_count             : 0x00000000 (0)
+                    __ndr_size               : 0xf8000001 (4160749569)
+                    first_object             : *
+                        first_object: struct drsuapi_DsReplicaObjectListItemEx
+                            next_object              : *
+                            object: struct drsuapi_DsReplicaObject
+                                identifier               : NULL
+                                flags                    : 0x3f000000 (1056964608)
+                                       0: DRSUAPI_DS_REPLICA_OBJECT_FROM_MASTER
+                                       0: DRSUAPI_DS_REPLICA_OBJECT_DYNAMIC
+                                       0: DRSUAPI_DS_REPLICA_OBJECT_REMOTE_MODIFY
+                                attribute_ctr: struct drsuapi_DsReplicaAttributeCtr
+                                    num_attributes           : 0x00000000 (0)
+                                    attributes               : NULL
+                            is_nc_prefix             : 0x00000000 (0)
+                            parent_object_guid       : NULL
+                            meta_data_ctr            : *
+                                meta_data_ctr: struct drsuapi_DsReplicaMetaDataCtr
+                                    count                    : 0x00000000 (0)
+                                    meta_data: ARRAY(0)
+                        next_object: struct drsuapi_DsReplicaObjectListItemEx
+                            next_object              : NULL
+                            object: struct drsuapi_DsReplicaObject
+                                identifier               : NULL
+                                flags                    : 0x00100006 (1048582)
+                                       0: DRSUAPI_DS_REPLICA_OBJECT_FROM_MASTER
+                                       1: DRSUAPI_DS_REPLICA_OBJECT_DYNAMIC
+                                       0: DRSUAPI_DS_REPLICA_OBJECT_REMOTE_MODIFY
+                                attribute_ctr: struct drsuapi_DsReplicaAttributeCtr
+                                    num_attributes           : 0x00000001 (1)
+                                    attributes               : *
+                                        attributes: ARRAY(1)
+                                            attributes: struct drsuapi_DsReplicaAttribute
+                                                attid                    : DRSUAPI_ATTID_cn (0x3)
+                                                value_ctr: struct drsuapi_DsAttributeValueCtr
+                                                    num_values               : 0x00000001 (1)
+                                                    values                   : *
+                                                        values: ARRAY(1)
+                                                            values: struct drsuapi_DsAttributeValue
+                                                                string                   : 'NULL'
+                            is_nc_prefix             : 0x00000001 (1)
+                            parent_object_guid       : *
+                                parent_object_guid       : 00100006-0001-0008-0100-000000000000
+                            meta_data_ctr            : *
+                                meta_data_ctr: struct drsuapi_DsReplicaMetaDataCtr
+                                    count                    : 0x00000000 (0)
+                                    meta_data: ARRAY(0)
+                    more_data                : 0x00000000 (0)
+            result                   : DOS code 0x00000100
+dump OK
diff --git a/source4/librpc/wscript_build b/source4/librpc/wscript_build
index c0ba7613273..009b2e13d2e 100644
--- a/source4/librpc/wscript_build
+++ b/source4/librpc/wscript_build
@@ -5,12 +5,6 @@ bld.RECURSE('../../librpc/tools')
 bld.RECURSE('idl')
 
 
-bld.SAMBA_SUBSYSTEM('NDR_WINSTATION',
-	source='gen_ndr/ndr_winstation.c',
-	public_deps='ndr'
-	)
-
-
 bld.SAMBA_SUBSYSTEM('NDR_IRPC',
 	source='gen_ndr/ndr_irpc.c',
 	public_deps='ndr NDR_SECURITY ndr_nbt'


-- 
Samba Shared Repository



More information about the samba-cvs mailing list