libfido2: Enable msan

[maflcko]

No description provided.

[github-actions]

maflcko has previously contributed to projects/libfido2. The previous PR was #11714

[maflcko]

cc @LDVG My understanding is that the msan errors produced here are true positives, albeit possibly harmless.

The one in fuzz_attobj can be fixed by initializing the memory, for example with zeros:

diff --git a/fuzz/fuzz_attobj.c b/fuzz/fuzz_attobj.c
index 403c096..7b7e071 100644
--- a/fuzz/fuzz_attobj.c
+++ b/fuzz/fuzz_attobj.c
@@ -243,7 +243,7 @@ fail:
 size_t
 pack_dummy(uint8_t *ptr, size_t len)
 {
-	struct param dummy;
+	struct param dummy={0};
 	uint8_t blob[MAXCORPUS];
 	size_t blob_len;
 

The others are possibly due to the use of NO_MSAN?

[LDVG]

Thank you for the heads-up. At a glance, these are failures local to the implementation of the harness itself. I'll test this locally and see what we can do to remedy.

[LDVG]

Hi again!

I agree, initializing dummy seems like the way to go for fuzz_attobj. In the other cases, it looks like some of our helper mutator functions were not expecting LLVMFuzzerMutate() to return uninitialised memory. I believe something along the lines of the below would do the trick:

diff --git a/fuzz/fuzz_attobj.c b/fuzz/fuzz_attobj.c
index 403c0966..4fddc0f4 100644
--- a/fuzz/fuzz_attobj.c
+++ b/fuzz/fuzz_attobj.c
@@ -247,6 +247,7 @@ pack_dummy(uint8_t *ptr, size_t len)
 	uint8_t blob[MAXCORPUS];
 	size_t blob_len;
 
+	memset(&dummy, 0, sizeof(dummy));
 	dummy.type = 1;
 
 	strlcpy(dummy.rp_id, dummy_rp_id, sizeof(dummy.rp_id));
diff --git a/fuzz/mutator_aux.c b/fuzz/mutator_aux.c
index 64c633f1..ebddf104 100644
--- a/fuzz/mutator_aux.c
+++ b/fuzz/mutator_aux.c
@@ -135,12 +135,18 @@ void
 mutate_byte(uint8_t *b)
 {
 	LLVMFuzzerMutate(b, sizeof(*b), sizeof(*b));
+#ifdef WITH_MSAN
+	__msan_unpoison(b, sizeof(*b));
+#endif
 }
 
 void
 mutate_int(int *i)
 {
 	LLVMFuzzerMutate((uint8_t *)i, sizeof(*i), sizeof(*i));
+#ifdef WITH_MSAN
+	__msan_unpoison(i, sizeof(*i));
+#endif
 }
 
 void

I'm hoping to push this (or a another fix, if this sounds incorrect) to libfido2's main branch on Friday.

[maflcko]

Sure, sounds good.

Just as an alternative, it would also be possible to restore the clang-15 behavior by setting the fno-sanitize flag:

diff --git a/projects/libfido2/build.sh b/projects/libfido2/build.sh
index 0e481f11a..40c3799c3 100755
--- a/projects/libfido2/build.sh
+++ b/projects/libfido2/build.sh
@@ -14,7 +14,8 @@
 # limitations under the License.
 #
 ################################################################################
-
+CFLAGS="-fno-sanitize-memory-param-retval $CFLAGS"
+CXXFLAGS="-fno-sanitize-memory-param-retval $CXXFLAGS"
 # Build libcbor, taken from oss-fuzz/projects/libcbor/build.sh
 # Note SANITIZE=OFF since it gets taken care of by $CFLAGS set by oss-fuzz
 cd ${SRC}/libcbor

But it seems better to limit the workarounds to just the affected functions. So your patch looks good.

[LDVG]

FYI, the above patch was merged to libfido2's main branch.

[maflcko]

Nice. The CI here is passing as well. Could you please approve this change, so that the OSS-Fuzz maintainers can merge it?