From 93b575b26605c347a717b2aa24ddf9cad08b8080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 7 Jul 2018 17:43:40 +0200 Subject: [PATCH 01/28] fuzz: rename "fuzz-corpus" directory to just "fuzz" Also, all corpus subdirectories are named exactly the same as the fuzzer they are for. This makes the paths a bit longer, but easier. --- test/{fuzz-corpus => fuzz}/.gitattributes | 0 .../fuzz-dhcp-server}/discover-existing | Bin .../fuzz-dhcp-server}/discover-new | Bin .../dhcp-server => fuzz/fuzz-dhcp-server}/release | Bin .../fuzz-dhcp-server}/request-existing | Bin .../fuzz-dhcp-server}/request-new | Bin .../fuzz-dhcp-server}/request-reboot | Bin .../fuzz-dhcp-server}/request-renew | Bin .../fuzz-journal-remote}/invalid-ts.txt | Bin .../fuzz-journal-remote}/sample.txt | 0 .../dev-mapper-fedora_krowka\\x2dswap.swap" | 0 .../fuzz-unit-file}/directives.service | 0 .../unit-file => fuzz/fuzz-unit-file}/empty.scope | 0 .../unit-file => fuzz/fuzz-unit-file}/machine.slice | 0 .../proc-sys-fs-binfmt_misc.automount | 0 .../unit-file => fuzz/fuzz-unit-file}/syslog.socket | 0 .../systemd-ask-password-console.path | 0 .../fuzz-unit-file}/systemd-machined.service | 0 .../fuzz-unit-file}/systemd-resolved.service | 0 .../fuzz-unit-file}/systemd-tmpfiles-clean.timer | 0 .../unit-file => fuzz/fuzz-unit-file}/timers.target | 0 .../fuzz-unit-file}/var-lib-machines.mount | 0 tools/oss-fuzz.sh | 6 ++++-- 23 files changed, 4 insertions(+), 2 deletions(-) rename test/{fuzz-corpus => fuzz}/.gitattributes (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/discover-existing (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/discover-new (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/release (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/request-existing (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/request-new (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/request-reboot (100%) rename test/{fuzz-corpus/dhcp-server => fuzz/fuzz-dhcp-server}/request-renew (100%) rename test/{fuzz-corpus/journal-remote => fuzz/fuzz-journal-remote}/invalid-ts.txt (100%) rename test/{fuzz-corpus/journal-remote => fuzz/fuzz-journal-remote}/sample.txt (100%) rename "test/fuzz-corpus/unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" => "test/fuzz/fuzz-unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/directives.service (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/empty.scope (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/machine.slice (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/proc-sys-fs-binfmt_misc.automount (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/syslog.socket (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/systemd-ask-password-console.path (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/systemd-machined.service (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/systemd-resolved.service (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/systemd-tmpfiles-clean.timer (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/timers.target (100%) rename test/{fuzz-corpus/unit-file => fuzz/fuzz-unit-file}/var-lib-machines.mount (100%) diff --git a/test/fuzz-corpus/.gitattributes b/test/fuzz/.gitattributes similarity index 100% rename from test/fuzz-corpus/.gitattributes rename to test/fuzz/.gitattributes diff --git a/test/fuzz-corpus/dhcp-server/discover-existing b/test/fuzz/fuzz-dhcp-server/discover-existing similarity index 100% rename from test/fuzz-corpus/dhcp-server/discover-existing rename to test/fuzz/fuzz-dhcp-server/discover-existing diff --git a/test/fuzz-corpus/dhcp-server/discover-new b/test/fuzz/fuzz-dhcp-server/discover-new similarity index 100% rename from test/fuzz-corpus/dhcp-server/discover-new rename to test/fuzz/fuzz-dhcp-server/discover-new diff --git a/test/fuzz-corpus/dhcp-server/release b/test/fuzz/fuzz-dhcp-server/release similarity index 100% rename from test/fuzz-corpus/dhcp-server/release rename to test/fuzz/fuzz-dhcp-server/release diff --git a/test/fuzz-corpus/dhcp-server/request-existing b/test/fuzz/fuzz-dhcp-server/request-existing similarity index 100% rename from test/fuzz-corpus/dhcp-server/request-existing rename to test/fuzz/fuzz-dhcp-server/request-existing diff --git a/test/fuzz-corpus/dhcp-server/request-new b/test/fuzz/fuzz-dhcp-server/request-new similarity index 100% rename from test/fuzz-corpus/dhcp-server/request-new rename to test/fuzz/fuzz-dhcp-server/request-new diff --git a/test/fuzz-corpus/dhcp-server/request-reboot b/test/fuzz/fuzz-dhcp-server/request-reboot similarity index 100% rename from test/fuzz-corpus/dhcp-server/request-reboot rename to test/fuzz/fuzz-dhcp-server/request-reboot diff --git a/test/fuzz-corpus/dhcp-server/request-renew b/test/fuzz/fuzz-dhcp-server/request-renew similarity index 100% rename from test/fuzz-corpus/dhcp-server/request-renew rename to test/fuzz/fuzz-dhcp-server/request-renew diff --git a/test/fuzz-corpus/journal-remote/invalid-ts.txt b/test/fuzz/fuzz-journal-remote/invalid-ts.txt similarity index 100% rename from test/fuzz-corpus/journal-remote/invalid-ts.txt rename to test/fuzz/fuzz-journal-remote/invalid-ts.txt diff --git a/test/fuzz-corpus/journal-remote/sample.txt b/test/fuzz/fuzz-journal-remote/sample.txt similarity index 100% rename from test/fuzz-corpus/journal-remote/sample.txt rename to test/fuzz/fuzz-journal-remote/sample.txt diff --git "a/test/fuzz-corpus/unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" "b/test/fuzz/fuzz-unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" similarity index 100% rename from "test/fuzz-corpus/unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" rename to "test/fuzz/fuzz-unit-file/dev-mapper-fedora_krowka\\x2dswap.swap" diff --git a/test/fuzz-corpus/unit-file/directives.service b/test/fuzz/fuzz-unit-file/directives.service similarity index 100% rename from test/fuzz-corpus/unit-file/directives.service rename to test/fuzz/fuzz-unit-file/directives.service diff --git a/test/fuzz-corpus/unit-file/empty.scope b/test/fuzz/fuzz-unit-file/empty.scope similarity index 100% rename from test/fuzz-corpus/unit-file/empty.scope rename to test/fuzz/fuzz-unit-file/empty.scope diff --git a/test/fuzz-corpus/unit-file/machine.slice b/test/fuzz/fuzz-unit-file/machine.slice similarity index 100% rename from test/fuzz-corpus/unit-file/machine.slice rename to test/fuzz/fuzz-unit-file/machine.slice diff --git a/test/fuzz-corpus/unit-file/proc-sys-fs-binfmt_misc.automount b/test/fuzz/fuzz-unit-file/proc-sys-fs-binfmt_misc.automount similarity index 100% rename from test/fuzz-corpus/unit-file/proc-sys-fs-binfmt_misc.automount rename to test/fuzz/fuzz-unit-file/proc-sys-fs-binfmt_misc.automount diff --git a/test/fuzz-corpus/unit-file/syslog.socket b/test/fuzz/fuzz-unit-file/syslog.socket similarity index 100% rename from test/fuzz-corpus/unit-file/syslog.socket rename to test/fuzz/fuzz-unit-file/syslog.socket diff --git a/test/fuzz-corpus/unit-file/systemd-ask-password-console.path b/test/fuzz/fuzz-unit-file/systemd-ask-password-console.path similarity index 100% rename from test/fuzz-corpus/unit-file/systemd-ask-password-console.path rename to test/fuzz/fuzz-unit-file/systemd-ask-password-console.path diff --git a/test/fuzz-corpus/unit-file/systemd-machined.service b/test/fuzz/fuzz-unit-file/systemd-machined.service similarity index 100% rename from test/fuzz-corpus/unit-file/systemd-machined.service rename to test/fuzz/fuzz-unit-file/systemd-machined.service diff --git a/test/fuzz-corpus/unit-file/systemd-resolved.service b/test/fuzz/fuzz-unit-file/systemd-resolved.service similarity index 100% rename from test/fuzz-corpus/unit-file/systemd-resolved.service rename to test/fuzz/fuzz-unit-file/systemd-resolved.service diff --git a/test/fuzz-corpus/unit-file/systemd-tmpfiles-clean.timer b/test/fuzz/fuzz-unit-file/systemd-tmpfiles-clean.timer similarity index 100% rename from test/fuzz-corpus/unit-file/systemd-tmpfiles-clean.timer rename to test/fuzz/fuzz-unit-file/systemd-tmpfiles-clean.timer diff --git a/test/fuzz-corpus/unit-file/timers.target b/test/fuzz/fuzz-unit-file/timers.target similarity index 100% rename from test/fuzz-corpus/unit-file/timers.target rename to test/fuzz/fuzz-unit-file/timers.target diff --git a/test/fuzz-corpus/unit-file/var-lib-machines.mount b/test/fuzz/fuzz-unit-file/var-lib-machines.mount similarity index 100% rename from test/fuzz-corpus/unit-file/var-lib-machines.mount rename to test/fuzz/fuzz-unit-file/var-lib-machines.mount diff --git a/tools/oss-fuzz.sh b/tools/oss-fuzz.sh index 2db5b4cc44..4d11e81ed6 100755 --- a/tools/oss-fuzz.sh +++ b/tools/oss-fuzz.sh @@ -35,8 +35,10 @@ fi meson $build -D$fuzzflag -Db_lundef=false ninja -C $build fuzzers -for d in "$(dirname "$0")/../test/fuzz-corpus/"*; do - zip -jqr $OUT/fuzz-$(basename "$d")_seed_corpus.zip "$d" +# The seed corpus is a separate flat archive for each fuzzer, +# with a fixed name ${fuzzer}_seed_corpus.zip. +for d in "$(dirname "$0")/../test/fuzz/fuzz-"*; do + zip -jqr $OUT/$(basename "$d")_seed_corpus.zip "$d" done # get fuzz-dns-packet corpus From c74a3f973e3e0bac13d66a28728a47f10046b71f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 7 Jul 2018 18:09:21 +0200 Subject: [PATCH 02/28] fuzz: unify the "fuzz-regressions" directory with the main corpus There isn't really much need to keep them separate. Anything which is a good corpus entry can be used as a smoke test, and anything which which is a regression test can just as well be inserted into the corpus. The only functional difference from this patch (apart from different paths in output) is that the regression tests are now zipped together with the rest of the corpus. $ meson configure build -Dslow-tests=true && ninja -C build test ... 307/325 fuzz-dns-packet:issue-7888:address OK 0.06 s 308/325 fuzz-dns-packet:oss-fuzz-5465:address OK 0.04 s 309/325 fuzz-journal-remote:crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76:address OK 0.07 s 310/325 fuzz-journal-remote:crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45:address OK 0.05 s 311/325 fuzz-journal-remote:oss-fuzz-8659:address OK 0.05 s 312/325 fuzz-journal-remote:oss-fuzz-8686:address OK 0.07 s 313/325 fuzz-unit-file:oss-fuzz-6884:address OK 0.06 s 314/325 fuzz-unit-file:oss-fuzz-6885:address OK 0.05 s 315/325 fuzz-unit-file:oss-fuzz-6886:address OK 0.05 s 316/325 fuzz-unit-file:oss-fuzz-6892:address OK 0.05 s 317/325 fuzz-unit-file:oss-fuzz-6897:address OK 0.05 s 318/325 fuzz-unit-file:oss-fuzz-6897-evverx:address OK 0.06 s 319/325 fuzz-unit-file:oss-fuzz-6908:address OK 0.07 s 320/325 fuzz-unit-file:oss-fuzz-6917:address OK 0.07 s 321/325 fuzz-unit-file:oss-fuzz-6977:address OK 0.13 s 322/325 fuzz-unit-file:oss-fuzz-6977-unminimized:address OK 0.12 s 323/325 fuzz-unit-file:oss-fuzz-7004:address OK 0.05 s 324/325 fuzz-unit-file:oss-fuzz-8064:address OK 0.05 s 325/325 fuzz-unit-file:oss-fuzz-8827:address OK 0.52 s --- meson.build | 4 +--- test/fuzz-regressions/.gitattributes | 1 - .../crash-4003c06fce43a11fbd22f02584df2807ac333eae | Bin .../crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef | Bin .../fuzz-dns-packet/issue-7888 | Bin .../fuzz-dns-packet/oss-fuzz-5465 | Bin .../crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 | Bin .../crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 | Bin .../fuzz-journal-remote/oss-fuzz-8659 | 0 .../fuzz-journal-remote/oss-fuzz-8686 | 0 .../fuzz-journald-syslog/github-9795 | 0 .../fuzz-journald-syslog/github-9820 | 0 .../fuzz-journald-syslog/github-9827 | 0 .../fuzz-journald-syslog/github-9829 | 0 ...timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 | Bin ...timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b | Bin .../fuzz-unit-file/oss-fuzz-10007 | 0 .../fuzz-unit-file/oss-fuzz-6884 | 0 .../fuzz-unit-file/oss-fuzz-6885 | 0 .../fuzz-unit-file/oss-fuzz-6886 | 0 .../fuzz-unit-file/oss-fuzz-6892 | 0 .../fuzz-unit-file/oss-fuzz-6897 | 0 .../fuzz-unit-file/oss-fuzz-6897-evverx | 0 .../fuzz-unit-file/oss-fuzz-6908 | 0 .../fuzz-unit-file/oss-fuzz-6917 | 0 .../fuzz-unit-file/oss-fuzz-6977 | 0 .../fuzz-unit-file/oss-fuzz-6977-unminimized | 0 .../fuzz-unit-file/oss-fuzz-7004 | 0 .../fuzz-unit-file/oss-fuzz-8064 | 0 .../fuzz-unit-file/oss-fuzz-8827 | 0 test/{fuzz-regressions => fuzz}/meson.build | 0 test/meson.build | 2 +- 32 files changed, 2 insertions(+), 5 deletions(-) delete mode 100644 test/fuzz-regressions/.gitattributes rename test/{fuzz-regressions => fuzz}/fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae (100%) rename test/{fuzz-regressions => fuzz}/fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef (100%) rename test/{fuzz-regressions => fuzz}/fuzz-dns-packet/issue-7888 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-dns-packet/oss-fuzz-5465 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journal-remote/oss-fuzz-8659 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journal-remote/oss-fuzz-8686 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journald-syslog/github-9795 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journald-syslog/github-9820 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journald-syslog/github-9827 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-journald-syslog/github-9829 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-10007 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6884 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6885 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6886 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6892 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6897 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6897-evverx (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6908 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6917 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6977 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-6977-unminimized (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-7004 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-8064 (100%) rename test/{fuzz-regressions => fuzz}/fuzz-unit-file/oss-fuzz-8827 (100%) rename test/{fuzz-regressions => fuzz}/meson.build (100%) diff --git a/meson.build b/meson.build index c29f622143..31d73aacfa 100644 --- a/meson.build +++ b/meson.build @@ -2847,9 +2847,7 @@ foreach tuple : sanitizers test('@0@:@1@:@2@'.format(b, c, sanitizer), env, args : [exe.full_path(), - join_paths(meson.source_root(), - 'test/fuzz-regressions', - p)]) + join_paths(meson.source_root(), 'test/fuzz', p)]) endif endforeach endif diff --git a/test/fuzz-regressions/.gitattributes b/test/fuzz-regressions/.gitattributes deleted file mode 100644 index 7b1b3e1835..0000000000 --- a/test/fuzz-regressions/.gitattributes +++ /dev/null @@ -1 +0,0 @@ -/*/* -whitespace diff --git a/test/fuzz-regressions/fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae b/test/fuzz/fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae similarity index 100% rename from test/fuzz-regressions/fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae rename to test/fuzz/fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae diff --git a/test/fuzz-regressions/fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef b/test/fuzz/fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef similarity index 100% rename from test/fuzz-regressions/fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef rename to test/fuzz/fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef diff --git a/test/fuzz-regressions/fuzz-dns-packet/issue-7888 b/test/fuzz/fuzz-dns-packet/issue-7888 similarity index 100% rename from test/fuzz-regressions/fuzz-dns-packet/issue-7888 rename to test/fuzz/fuzz-dns-packet/issue-7888 diff --git a/test/fuzz-regressions/fuzz-dns-packet/oss-fuzz-5465 b/test/fuzz/fuzz-dns-packet/oss-fuzz-5465 similarity index 100% rename from test/fuzz-regressions/fuzz-dns-packet/oss-fuzz-5465 rename to test/fuzz/fuzz-dns-packet/oss-fuzz-5465 diff --git a/test/fuzz-regressions/fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 b/test/fuzz/fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 similarity index 100% rename from test/fuzz-regressions/fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 rename to test/fuzz/fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 diff --git a/test/fuzz-regressions/fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 b/test/fuzz/fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 similarity index 100% rename from test/fuzz-regressions/fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 rename to test/fuzz/fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 diff --git a/test/fuzz-regressions/fuzz-journal-remote/oss-fuzz-8659 b/test/fuzz/fuzz-journal-remote/oss-fuzz-8659 similarity index 100% rename from test/fuzz-regressions/fuzz-journal-remote/oss-fuzz-8659 rename to test/fuzz/fuzz-journal-remote/oss-fuzz-8659 diff --git a/test/fuzz-regressions/fuzz-journal-remote/oss-fuzz-8686 b/test/fuzz/fuzz-journal-remote/oss-fuzz-8686 similarity index 100% rename from test/fuzz-regressions/fuzz-journal-remote/oss-fuzz-8686 rename to test/fuzz/fuzz-journal-remote/oss-fuzz-8686 diff --git a/test/fuzz-regressions/fuzz-journald-syslog/github-9795 b/test/fuzz/fuzz-journald-syslog/github-9795 similarity index 100% rename from test/fuzz-regressions/fuzz-journald-syslog/github-9795 rename to test/fuzz/fuzz-journald-syslog/github-9795 diff --git a/test/fuzz-regressions/fuzz-journald-syslog/github-9820 b/test/fuzz/fuzz-journald-syslog/github-9820 similarity index 100% rename from test/fuzz-regressions/fuzz-journald-syslog/github-9820 rename to test/fuzz/fuzz-journald-syslog/github-9820 diff --git a/test/fuzz-regressions/fuzz-journald-syslog/github-9827 b/test/fuzz/fuzz-journald-syslog/github-9827 similarity index 100% rename from test/fuzz-regressions/fuzz-journald-syslog/github-9827 rename to test/fuzz/fuzz-journald-syslog/github-9827 diff --git a/test/fuzz-regressions/fuzz-journald-syslog/github-9829 b/test/fuzz/fuzz-journald-syslog/github-9829 similarity index 100% rename from test/fuzz-regressions/fuzz-journald-syslog/github-9829 rename to test/fuzz/fuzz-journald-syslog/github-9829 diff --git a/test/fuzz-regressions/fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 b/test/fuzz/fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 similarity index 100% rename from test/fuzz-regressions/fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 rename to test/fuzz/fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 diff --git a/test/fuzz-regressions/fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b b/test/fuzz/fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b similarity index 100% rename from test/fuzz-regressions/fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b rename to test/fuzz/fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-10007 b/test/fuzz/fuzz-unit-file/oss-fuzz-10007 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-10007 rename to test/fuzz/fuzz-unit-file/oss-fuzz-10007 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 b/test/fuzz/fuzz-unit-file/oss-fuzz-6884 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6884 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 b/test/fuzz/fuzz-unit-file/oss-fuzz-6885 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6885 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 b/test/fuzz/fuzz-unit-file/oss-fuzz-6886 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6886 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6892 b/test/fuzz/fuzz-unit-file/oss-fuzz-6892 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6892 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6892 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897 b/test/fuzz/fuzz-unit-file/oss-fuzz-6897 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6897 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897-evverx b/test/fuzz/fuzz-unit-file/oss-fuzz-6897-evverx similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897-evverx rename to test/fuzz/fuzz-unit-file/oss-fuzz-6897-evverx diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6908 b/test/fuzz/fuzz-unit-file/oss-fuzz-6908 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6908 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6908 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6917 b/test/fuzz/fuzz-unit-file/oss-fuzz-6917 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6917 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6917 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6977 b/test/fuzz/fuzz-unit-file/oss-fuzz-6977 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6977 rename to test/fuzz/fuzz-unit-file/oss-fuzz-6977 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6977-unminimized b/test/fuzz/fuzz-unit-file/oss-fuzz-6977-unminimized similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6977-unminimized rename to test/fuzz/fuzz-unit-file/oss-fuzz-6977-unminimized diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-7004 b/test/fuzz/fuzz-unit-file/oss-fuzz-7004 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-7004 rename to test/fuzz/fuzz-unit-file/oss-fuzz-7004 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-8064 b/test/fuzz/fuzz-unit-file/oss-fuzz-8064 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-8064 rename to test/fuzz/fuzz-unit-file/oss-fuzz-8064 diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-8827 b/test/fuzz/fuzz-unit-file/oss-fuzz-8827 similarity index 100% rename from test/fuzz-regressions/fuzz-unit-file/oss-fuzz-8827 rename to test/fuzz/fuzz-unit-file/oss-fuzz-8827 diff --git a/test/fuzz-regressions/meson.build b/test/fuzz/meson.build similarity index 100% rename from test/fuzz-regressions/meson.build rename to test/fuzz/meson.build diff --git a/test/meson.build b/test/meson.build index 9750ff22b9..37034fca96 100644 --- a/test/meson.build +++ b/test/meson.build @@ -256,4 +256,4 @@ if conf.get('ENABLE_HWDB') == 1 endif endif -subdir('fuzz-regressions') +subdir('fuzz') From 3ddf3d439463ab2c76391a4d22b54166be2dbe94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 7 Jul 2018 19:08:52 +0200 Subject: [PATCH 03/28] test-bus-marshal: use cescaping instead of hexmem It is easier to see the contents this way by eye. --- src/libsystemd/sd-bus/test-bus-marshal.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 020a5e51e5..9e09b8f61c 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/src/libsystemd/sd-bus/test-bus-marshal.c @@ -18,8 +18,8 @@ #include "bus-label.h" #include "bus-message.h" #include "bus-util.h" +#include "escape.h" #include "fd-util.h" -#include "hexdecoct.h" #include "log.h" #include "tests.h" #include "util.h" @@ -111,7 +111,7 @@ int main(int argc, char *argv[]) { uint8_t u, v; void *buffer = NULL; size_t sz; - char *h; + _cleanup_free_ char *h = NULL; const int32_t integer_array[] = { -1, -2, 0, 1, 2 }, *return_array; char *s; _cleanup_free_ char *first = NULL, *second = NULL, *third = NULL; @@ -197,11 +197,9 @@ int main(int argc, char *argv[]) { r = bus_message_get_blob(m, &buffer, &sz); assert_se(r >= 0); - h = hexmem(buffer, sz); + h = cescape_length(buffer, sz); assert_se(h); - log_info("message size = %zu, contents =\n%s", sz, h); - free(h); #if HAVE_GLIB #ifndef __SANITIZE_ADDRESS__ From 6bd2bc8e16a6d515f8a21c47fd6b833d7fcfdd1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Aug 2018 18:10:53 +0200 Subject: [PATCH 04/28] meson: drop duplicated condition The generic check suffices for those four. --- meson.build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index 31d73aacfa..865159ca3c 100644 --- a/meson.build +++ b/meson.build @@ -3047,10 +3047,10 @@ foreach tuple : [ ['blkid'], ['dbus'], ['glib'], - ['nss-myhostname', conf.get('ENABLE_NSS_MYHOSTNAME') == 1], - ['nss-mymachines', conf.get('ENABLE_NSS_MYMACHINES') == 1], - ['nss-resolve', conf.get('ENABLE_NSS_RESOLVE') == 1], - ['nss-systemd', conf.get('ENABLE_NSS_SYSTEMD') == 1], + ['nss-myhostname'], + ['nss-mymachines'], + ['nss-resolve'], + ['nss-systemd'], ['hwdb'], ['tpm'], ['man pages', want_man], From fd5dec9adf76591d713f163d43d04e3beb76893e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Aug 2018 17:34:47 +0200 Subject: [PATCH 05/28] meson: add -Dlog-trace to set LOG_TRACE The justification is the same as for -Dvalgrind: setting config in meson in this way is easier, because when the value is changed stuff that should be rebuilt is rebuilt. --- meson.build | 2 ++ meson_options.txt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/meson.build b/meson.build index 865159ca3c..a7d48a2c45 100644 --- a/meson.build +++ b/meson.build @@ -781,6 +781,7 @@ conf.set10('ENABLE_DEBUG_HASHMAP', enable_debug_hashmap) conf.set10('ENABLE_DEBUG_MMAP_CACHE', enable_debug_mmap_cache) conf.set10('VALGRIND', get_option('valgrind')) +conf.set10('LOG_TRACE', get_option('log-trace')) ##################################################################### @@ -3066,6 +3067,7 @@ foreach tuple : [ ['debug hashmap'], ['debug mmap cache'], ['valgrind', conf.get('VALGRIND') == 1], + ['trace logging', conf.get('LOG_TRACE') == 1], ] if tuple.length() >= 2 diff --git a/meson_options.txt b/meson_options.txt index 0407b97b5b..83ade5bea4 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -51,6 +51,8 @@ option('memory-accounting-default', type : 'boolean', description : 'enable MemoryAccounting= by default') option('valgrind', type : 'boolean', value : false, description : 'do extra operations to avoid valgrind warnings') +option('log-trace', type : 'boolean', value : false, + description : 'enable low level debug logging') option('utmp', type : 'boolean', description : 'support for utmp/wtmp log handling') From 243e5cecc3a211519544ccba01c44edc827ac517 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Aug 2018 16:50:07 +0200 Subject: [PATCH 06/28] meson: use .source_root() in more places In the main meson.build file, .source_root() and .current_source_dir() are equivalent, but it seems more appropriate to use .source_root() when we are appending a path which is by design relative to repo root. --- meson.build | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/meson.build b/meson.build index a7d48a2c45..2de27861de 100644 --- a/meson.build +++ b/meson.build @@ -1477,7 +1477,7 @@ foreach tuple : [['myhostname', 'ENABLE_NSS_MYHOSTNAME'], module = tuple[0] sym = 'src/nss-@0@/nss-@0@.sym'.format(module) - version_script_arg = join_paths(meson.current_source_dir(), sym) + version_script_arg = join_paths(meson.source_root(), sym) nss = shared_library( 'nss_' + module, @@ -1732,7 +1732,7 @@ if conf.get('ENABLE_LOGIND') == 1 public_programs += exe if conf.get('HAVE_PAM') == 1 - version_script_arg = join_paths(meson.current_source_dir(), pam_systemd_sym) + version_script_arg = join_paths(meson.source_root(), pam_systemd_sym) pam_systemd = shared_library( 'pam_systemd', pam_systemd_c, @@ -2860,7 +2860,7 @@ endforeach if git.found() all_files = run_command( git, - ['--git-dir=@0@/.git'.format(meson.current_source_dir()), + ['--git-dir=@0@/.git'.format(meson.source_root()), 'ls-files', ':/*.[ch]']) all_files = files(all_files.stdout().split()) @@ -2868,10 +2868,10 @@ if git.found() custom_target( 'tags', output : 'tags', - command : [env, 'etags', '-o', '@0@/TAGS'.format(meson.current_source_dir())] + all_files) + command : [env, 'etags', '-o', '@0@/TAGS'.format(meson.source_root())] + all_files) run_target( 'ctags', - command : [env, 'ctags', '-o', '@0@/tags'.format(meson.current_source_dir())] + all_files) + command : [env, 'ctags', '-o', '@0@/tags'.format(meson.source_root())] + all_files) endif if git.found() @@ -2884,17 +2884,17 @@ endif if git.found() git_head = run_command( git, - ['--git-dir=@0@/.git'.format(meson.current_source_dir()), + ['--git-dir=@0@/.git'.format(meson.source_root()), 'rev-parse', 'HEAD']).stdout().strip() git_head_short = run_command( git, - ['--git-dir=@0@/.git'.format(meson.current_source_dir()), + ['--git-dir=@0@/.git'.format(meson.source_root()), 'rev-parse', '--short=7', 'HEAD']).stdout().strip() run_target( 'git-snapshot', command : ['git', 'archive', - '-o', '@0@/systemd-@1@.tar.gz'.format(meson.current_source_dir(), + '-o', '@0@/systemd-@1@.tar.gz'.format(meson.source_root(), git_head_short), '--prefix', 'systemd-@0@/'.format(git_head), 'HEAD']) From e6bad6746151c79a5f408e95714ffa5cea290ab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 10 Aug 2018 17:15:05 +0200 Subject: [PATCH 07/28] meson: treat all fuzz cases as unit tests 318/365 fuzz-bus-message:crash-26bba7182dedc8848939931d9fcefcb7922f2e56:address OK 0.03 s 319/365 fuzz-bus-message:crash-29ed3c202e0ffade3cad42c8bbeb6cc68a21eb8e:address OK 0.03 s 320/365 fuzz-bus-message:crash-b88ad9ecf4aacf4a0caca5b5543953265367f084:address OK 0.03 s 321/365 fuzz-bus-message:crash-c1b37b4729b42c0c05b23cba4eed5d8102498a1e:address OK 0.03 s 322/365 fuzz-bus-message:crash-d8f3941c74219b4c03532c9b244d5ea539c61af5:address OK 0.03 s 323/365 fuzz-bus-message:crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5:address OK 0.03 s 324/365 fuzz-bus-message:leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20:address OK 0.04 s 325/365 fuzz-bus-message:message1:address OK 0.03 s 326/365 fuzz-bus-message:timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b:address OK 0.03 s 327/365 fuzz-dhcp-server:discover-existing:address OK 0.04 s 328/365 fuzz-dhcp-server:discover-new:address OK 0.03 s 329/365 fuzz-dhcp-server:release:address OK 0.04 s 330/365 fuzz-dhcp-server:request-existing:address OK 0.03 s 331/365 fuzz-dhcp-server:request-new:address OK 0.03 s 332/365 fuzz-dhcp-server:request-reboot:address OK 0.03 s 333/365 fuzz-dhcp-server:request-renew:address OK 0.03 s 334/365 fuzz-dns-packet:issue-7888:address OK 0.03 s 335/365 fuzz-dns-packet:oss-fuzz-5465:address OK 0.03 s 336/365 fuzz-journal-remote:crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76:address OK 0.06 s 337/365 fuzz-journal-remote:crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45:address OK 0.04 s 338/365 fuzz-journal-remote:invalid-ts.txt:address OK 0.04 s 339/365 fuzz-journal-remote:oss-fuzz-8659:address OK 0.06 s 340/365 fuzz-journal-remote:oss-fuzz-8686:address OK 0.04 s 341/365 fuzz-journal-remote:sample.txt:address OK 0.07 s 342/365 fuzz-unit-file:directives.service:address OK 0.03 s 343/365 fuzz-unit-file:empty.scope:address OK 0.04 s 344/365 fuzz-unit-file:machine.slice:address OK 0.03 s 345/365 fuzz-unit-file:oss-fuzz-6884:address OK 0.05 s 346/365 fuzz-unit-file:oss-fuzz-6885:address OK 0.03 s 347/365 fuzz-unit-file:oss-fuzz-6886:address OK 0.04 s 348/365 fuzz-unit-file:oss-fuzz-6892:address OK 0.03 s 349/365 fuzz-unit-file:oss-fuzz-6897:address OK 0.05 s 350/365 fuzz-unit-file:oss-fuzz-6897-evverx:address OK 0.04 s 351/365 fuzz-unit-file:oss-fuzz-6908:address OK 0.05 s 352/365 fuzz-unit-file:oss-fuzz-6917:address OK 0.06 s 353/365 fuzz-unit-file:oss-fuzz-6977:address OK 0.08 s 354/365 fuzz-unit-file:oss-fuzz-6977-unminimized:address OK 0.10 s 355/365 fuzz-unit-file:oss-fuzz-7004:address OK 0.03 s 356/365 fuzz-unit-file:oss-fuzz-8064:address OK 0.03 s 357/365 fuzz-unit-file:oss-fuzz-8827:address OK 0.50 s 358/365 fuzz-unit-file:proc-sys-fs-binfmt_misc.automount:address OK 0.03 s 359/365 fuzz-unit-file:syslog.socket:address OK 0.03 s 360/365 fuzz-unit-file:systemd-ask-password-console.path:address OK 0.03 s 361/365 fuzz-unit-file:systemd-machined.service:address OK 0.03 s 362/365 fuzz-unit-file:systemd-resolved.service:address OK 0.03 s 363/365 fuzz-unit-file:systemd-tmpfiles-clean.timer:address OK 0.03 s 364/365 fuzz-unit-file:timers.target:address OK 0.03 s 365/365 fuzz-unit-file:var-lib-machines.mount:address OK 0.04 s This gives us slightly nicer coverage in the normal test run. When in a git repo, git ls-files is used to get a list of files known to git. This mirrors what update-man-rules does for man files. Only looking at files known to git makes it easier to not forget to commit the test file to git, and also makes bisecting easier if some files are left in repo. When outside of a git repo, we expect to be unpacked from a tarball, so just using all files reported by ls is OK. --- meson.build | 2 +- test/fuzz/meson.build | 50 +++++++++++++++++-------------------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/meson.build b/meson.build index 2de27861de..559c99af34 100644 --- a/meson.build +++ b/meson.build @@ -2848,7 +2848,7 @@ foreach tuple : sanitizers test('@0@:@1@:@2@'.format(b, c, sanitizer), env, args : [exe.full_path(), - join_paths(meson.source_root(), 'test/fuzz', p)]) + join_paths(meson.source_root(), p)]) endif endforeach endif diff --git a/test/fuzz/meson.build b/test/fuzz/meson.build index 043d3f3154..56d0f69660 100644 --- a/test/fuzz/meson.build +++ b/test/fuzz/meson.build @@ -11,33 +11,23 @@ sanitize_address = custom_target( sanitizers = [['address', sanitize_address]] -fuzz_regression_tests = ''' - fuzz-dhcp6-client/crash-4003c06fce43a11fbd22f02584df2807ac333eae - fuzz-dhcp6-client/crash-6e88fcb6b85c9436bcbe05219aa8e550194645ef - fuzz-dns-packet/issue-7888 - fuzz-dns-packet/oss-fuzz-5465 - fuzz-journal-remote/crash-5a8f03d4c3a46fcded39527084f437e8e4b54b76 - fuzz-journal-remote/crash-96dee870ea66d03e89ac321eee28ea63a9b9aa45 - fuzz-journal-remote/oss-fuzz-8659 - fuzz-journal-remote/oss-fuzz-8686 - fuzz-journald-syslog/github-9795 - fuzz-journald-syslog/github-9820 - fuzz-journald-syslog/github-9827 - fuzz-journald-syslog/github-9829 - fuzz-ndisc-rs/timeout-2815b773c712fa33bea62f541dfa3017c64ea2f1 - fuzz-ndisc-rs/timeout-61fff7fd1e5dcc07e1b656baab29065ce634ad5b - fuzz-unit-file/oss-fuzz-6884 - fuzz-unit-file/oss-fuzz-6885 - fuzz-unit-file/oss-fuzz-6886 - fuzz-unit-file/oss-fuzz-6892 - fuzz-unit-file/oss-fuzz-6897 - fuzz-unit-file/oss-fuzz-6897-evverx - fuzz-unit-file/oss-fuzz-6908 - fuzz-unit-file/oss-fuzz-6917 - fuzz-unit-file/oss-fuzz-6977 - fuzz-unit-file/oss-fuzz-6977-unminimized - fuzz-unit-file/oss-fuzz-7004 - fuzz-unit-file/oss-fuzz-8064 - fuzz-unit-file/oss-fuzz-8827 - fuzz-unit-file/oss-fuzz-10007 -'''.split() +if git.found() + out = run_command( + git, + '--git-dir=@0@/.git'.format(meson.source_root()), + 'ls-files', ':/test/fuzz/*/*') +else + out = run_command( + 'sh', '-c', 'ls @0@/*/*'.format(meson.current_source_dir())) +endif + +fuzz_regression_tests = [] +foreach p : out.stdout().split() + # Remove the last entry which is ''. + # + # Also, backslashes get mangled, so skip test. See + # https://github.com/mesonbuild/meson/issues/1564. + if not p.contains('\\') + fuzz_regression_tests += p + endif +endforeach From 56b560c26339c4b282c06038316a91509eae75fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 7 Jul 2018 19:30:25 +0200 Subject: [PATCH 08/28] fuzz-bus-message: add fuzzer for message parsing As with other fuzzers, SYSTEMD_FUZZ_OUTPUT=1 and SYSTEMD_LOG_LEVEL=debug can be used for debugging. --- src/fuzz/fuzz-bus-message.c | 47 ++++++++++++++++++++++++++++ src/fuzz/meson.build | 4 +++ test/fuzz/fuzz-bus-message/message1 | Bin 0 -> 534 bytes 3 files changed, 51 insertions(+) create mode 100644 src/fuzz/fuzz-bus-message.c create mode 100644 test/fuzz/fuzz-bus-message/message1 diff --git a/src/fuzz/fuzz-bus-message.c b/src/fuzz/fuzz-bus-message.c new file mode 100644 index 0000000000..9842c62a6f --- /dev/null +++ b/src/fuzz/fuzz-bus-message.c @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include + +#include "alloc-util.h" +#include "bus-dump.h" +#include "bus-message.h" +#include "env-util.h" +#include "fd-util.h" +#include "fuzz.h" + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { + _cleanup_free_ char *out = NULL; /* out should be freed after g */ + size_t out_size; + _cleanup_fclose_ FILE *g = NULL; + _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + _cleanup_free_ void *buffer = NULL; + int r; + + /* We don't want to fill the logs with messages about parse errors. + * Disable most logging if not running standalone */ + if (!getenv("SYSTEMD_LOG_LEVEL")) + log_set_max_level(LOG_CRIT); + + r = sd_bus_new(&bus); + assert_se(r >= 0); + + assert_se(buffer = memdup(data, size)); + + r = bus_message_from_malloc(bus, buffer, size, NULL, 0, NULL, &m); + if (r == -EBADMSG) + return 0; + assert_se(r >= 0); + TAKE_PTR(buffer); + + if (getenv_bool("SYSTEMD_FUZZ_OUTPUT") <= 0) + assert_se(g = open_memstream(&out, &out_size)); + + bus_message_dump(m, g ?: stdout, BUS_MESSAGE_DUMP_WITH_HEADER); + + r = sd_bus_message_rewind(m, true); + assert_se(r >= 0); + + return 0; +} diff --git a/src/fuzz/meson.build b/src/fuzz/meson.build index 066737c175..31ee41cbe0 100644 --- a/src/fuzz/meson.build +++ b/src/fuzz/meson.build @@ -1,6 +1,10 @@ # SPDX-License-Identifier: LGPL-2.1+ fuzzers += [ + [['src/fuzz/fuzz-bus-message.c'], + [libshared], + []], + [['src/fuzz/fuzz-dns-packet.c', dns_type_headers], [libsystemd_resolve_core, diff --git a/test/fuzz/fuzz-bus-message/message1 b/test/fuzz/fuzz-bus-message/message1 new file mode 100644 index 0000000000000000000000000000000000000000..2df70fd7cb6f0e632c4d5c2358091309a5cd3edc GIT binary patch literal 534 zcmZ{h!A`?442GSJjTUi2h$EV`OM6*iyZ|>&NW6m6ZC#~`RCNGV2*icg27V{4hLEuI z*Z%6nv6IG-c{fDW8PO*Z8RG~@1*A4LLPziq^|n=>fKTCf&ROnOFWhXL{-6KzKQR>* zA}kdo{MtXi^_lPUKI=U`x#dhG*Hq0w(L%415E=fPT+(I2*knx96tO2RVnnVP6o! Yuz#WfEX)Cqd!b_JHzYppZsXhk08nC8%>V!Z literal 0 HcmV?d00001 From 7f546026abbdc56c453a577e52d57159458c3e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 Jul 2018 07:03:01 +0200 Subject: [PATCH 09/28] Introduce free_and_strndup and use it in bus-message.c v2: fix error in free_and_strndup() When the orignal and copied message were the same, but shorter than specified length l, memory read past the end of the buffer would be performed. A test case is included: a string that had an embedded NUL ("q\0") is used to replace "q". v3: Fix one more bug in free_and_strndup and add tests. v4: Some style fixed based on review, one more use of free_and_replace, and make the tests more comprehensive. --- src/basic/string-util.c | 28 +++++++- src/basic/string-util.h | 1 + src/libsystemd/sd-bus/bus-message.c | 34 ++++------ src/test/test-string-util.c | 62 ++++++++++++++++++ ...h-b88ad9ecf4aacf4a0caca5b5543953265367f084 | Bin 0 -> 32 bytes 5 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 0a40683493..dfa739996f 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1004,7 +1004,7 @@ int free_and_strdup(char **p, const char *s) { assert(p); - /* Replaces a string pointer with an strdup()ed new string, + /* Replaces a string pointer with a strdup()ed new string, * possibly freeing the old one. */ if (streq_ptr(*p, s)) @@ -1023,6 +1023,32 @@ int free_and_strdup(char **p, const char *s) { return 1; } +int free_and_strndup(char **p, const char *s, size_t l) { + char *t; + + assert(p); + assert(s || l == 0); + + /* Replaces a string pointer with a strndup()ed new string, + * freeing the old one. */ + + if (!*p && !s) + return 0; + + if (*p && s && strneq(*p, s, l) && (l > strlen(*p) || (*p)[l] == '\0')) + return 0; + + if (s) { + t = strndup(s, l); + if (!t) + return -ENOMEM; + } else + t = NULL; + + free_and_replace(*p, t); + return 1; +} + #if !HAVE_EXPLICIT_BZERO /* * Pointer to memset is volatile so that compiler must de-reference diff --git a/src/basic/string-util.h b/src/basic/string-util.h index fcd12f3a30..72c075aa39 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -176,6 +176,7 @@ char *strrep(const char *s, unsigned n); int split_pair(const char *s, const char *sep, char **l, char **r); int free_and_strdup(char **p, const char *s); +int free_and_strndup(char **p, const char *s, size_t l); /* Normal memmem() requires haystack to be nonnull, which is annoying for zero-length buffers */ static inline void *memmem_safe(const void *haystack, size_t haystacklen, const void *needle, size_t needlelen) { diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 6e404367db..91bcf49a48 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4144,20 +4144,19 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (contents) { size_t l; - char *sig; r = signature_element_length(c->signature+c->index+1, &l); if (r < 0) return r; - assert(l >= 1); + /* signature_element_length does verification internally */ - sig = strndup(c->signature + c->index + 1, l); - if (!sig) + assert(l >= 1); + if (free_and_strndup(&c->peeked_signature, + c->signature + c->index + 1, l) < 0) return -ENOMEM; - free(c->peeked_signature); - *contents = c->peeked_signature = sig; + *contents = c->peeked_signature; } if (type) @@ -4170,19 +4169,17 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (contents) { size_t l; - char *sig; r = signature_element_length(c->signature+c->index, &l); if (r < 0) return r; assert(l >= 2); - sig = strndup(c->signature + c->index + 1, l - 2); - if (!sig) + if (free_and_strndup(&c->peeked_signature, + c->signature + c->index + 1, l - 2) < 0) return -ENOMEM; - free(c->peeked_signature); - *contents = c->peeked_signature = sig; + *contents = c->peeked_signature; } if (type) @@ -4222,9 +4219,8 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (k > c->item_size) return -EBADMSG; - free(c->peeked_signature); - c->peeked_signature = strndup((char*) q + 1, k - 1); - if (!c->peeked_signature) + if (free_and_strndup(&c->peeked_signature, + (char*) q + 1, k - 1) < 0) return -ENOMEM; if (!signature_is_valid(c->peeked_signature, true)) @@ -5056,25 +5052,21 @@ int bus_message_parse_fields(sd_bus_message *m) { if (*p == 0) { size_t l; - char *c; /* We found the beginning of the signature * string, yay! We require the body to be a * structure, so verify it and then strip the * opening/closing brackets. */ - l = ((char*) m->footer + m->footer_accessible) - p - (1 + sz); + l = (char*) m->footer + m->footer_accessible - p - (1 + sz); if (l < 2 || p[1] != SD_BUS_TYPE_STRUCT_BEGIN || p[1 + l - 1] != SD_BUS_TYPE_STRUCT_END) return -EBADMSG; - c = strndup(p + 1 + 1, l - 2); - if (!c) + if (free_and_strndup(&m->root_container.signature, + p + 1 + 1, l - 2) < 0) return -ENOMEM; - - free(m->root_container.signature); - m->root_container.signature = c; break; } diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index a9f261839c..8c1f91d4ef 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -5,6 +5,7 @@ #include "macro.h" #include "string-util.h" #include "strv.h" +#include "tests.h" #include "utf8.h" static void test_string_erase(void) { @@ -30,6 +31,64 @@ static void test_string_erase(void) { assert_se(x[9] == '\0'); } +static void test_free_and_strndup_one(char **t, const char *src, size_t l, const char *expected, bool change) { + int r; + + log_debug("%s: \"%s\", \"%s\", %zd (expect \"%s\", %s)", + __func__, strnull(*t), strnull(src), l, strnull(expected), yes_no(change)); + + r = free_and_strndup(t, src, l); + assert_se(streq_ptr(*t, expected)); + assert_se(r == change); /* check that change occurs only when necessary */ +} + +static void test_free_and_strndup(void) { + static const struct test_case { + const char *src; + size_t len; + const char *expected; + } cases[] = { + {"abc", 0, ""}, + {"abc", 0, ""}, + {"abc", 1, "a"}, + {"abc", 2, "ab"}, + {"abc", 3, "abc"}, + {"abc", 4, "abc"}, + {"abc", 5, "abc"}, + {"abc", 5, "abc"}, + {"abc", 4, "abc"}, + {"abc", 3, "abc"}, + {"abc", 2, "ab"}, + {"abc", 1, "a"}, + {"abc", 0, ""}, + + {"", 0, ""}, + {"", 1, ""}, + {"", 2, ""}, + {"", 0, ""}, + {"", 1, ""}, + {"", 2, ""}, + {"", 2, ""}, + {"", 1, ""}, + {"", 0, ""}, + + {NULL, 0, NULL}, + + {"foo", 3, "foo"}, + {"foobar", 6, "foobar"}, + }; + + _cleanup_free_ char *t = NULL; + const char *prev_expected = t; + + for (unsigned i = 0; i < ELEMENTSOF(cases); i++) { + test_free_and_strndup_one(&t, + cases[i].src, cases[i].len, cases[i].expected, + !streq_ptr(cases[i].expected, prev_expected)); + prev_expected = t; + } +} + static void test_ascii_strcasecmp_n(void) { assert_se(ascii_strcasecmp_n("", "", 0) == 0); @@ -520,7 +579,10 @@ static void test_memory_startswith_no_case(void) { } int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + test_string_erase(); + test_free_and_strndup(); test_ascii_strcasecmp_n(); test_ascii_strcasecmp_nn(); test_cellescape(); diff --git a/test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 b/test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 new file mode 100644 index 0000000000000000000000000000000000000000..52469650b5498a45d5d95bd9d933c989cfb47ca7 GIT binary patch literal 32 ccmd1#|DTBg0(2Mzp)7_%AVVXuuuM|`09r!?!~g&Q literal 0 HcmV?d00001 From cf81c68e96aa29d0c28b5d3a26d1de9aa1b53b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 Jul 2018 07:38:10 +0200 Subject: [PATCH 10/28] bus-message: use structured initialization to avoid use of unitialized memory As far as I can see, we would either reuse some values from a previously exited container or just random bytes from the heap. Should fix #10127. --- src/libsystemd/sd-bus/bus-message.c | 56 ++++++++++++++--------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 91bcf49a48..efbd3307c1 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -1924,7 +1924,7 @@ _public_ int sd_bus_message_open_container( char type, const char *contents) { - struct bus_container *c, *w; + struct bus_container *c; uint32_t *array_size = NULL; _cleanup_free_ char *signature = NULL; size_t before, begin = 0; @@ -1969,16 +1969,14 @@ _public_ int sd_bus_message_open_container( return r; /* OK, let's fill it in */ - w = m->containers + m->n_containers++; - w->enclosing = type; - w->signature = TAKE_PTR(signature); - w->index = 0; - w->array_size = array_size; - w->before = before; - w->begin = begin; - w->n_offsets = w->offsets_allocated = 0; - w->offsets = NULL; - w->need_offsets = need_offsets; + m->containers[m->n_containers++] = (struct bus_container) { + .enclosing = type, + .signature = TAKE_PTR(signature), + .array_size = array_size, + .before = before, + .begin = begin, + .need_offsets = need_offsets, + }; return 0; } @@ -3941,10 +3939,10 @@ static int bus_message_enter_dict_entry( _public_ int sd_bus_message_enter_container(sd_bus_message *m, char type, const char *contents) { - struct bus_container *c, *w; + struct bus_container *c; uint32_t *array_size = NULL; _cleanup_free_ char *signature = NULL; - size_t before; + size_t before, end; _cleanup_free_ size_t *offsets = NULL; size_t n_offsets = 0, item_size = 0; int r; @@ -4023,28 +4021,26 @@ _public_ int sd_bus_message_enter_container(sd_bus_message *m, return r; /* OK, let's fill it in */ - w = m->containers + m->n_containers++; - w->enclosing = type; - w->signature = TAKE_PTR(signature); - w->peeked_signature = NULL; - w->index = 0; - - w->before = before; - w->begin = m->rindex; - - /* Unary type has fixed size of 1, but virtual size of 0 */ if (BUS_MESSAGE_IS_GVARIANT(m) && type == SD_BUS_TYPE_STRUCT && isempty(signature)) - w->end = m->rindex + 0; + end = m->rindex + 0; else - w->end = m->rindex + c->item_size; + end = m->rindex + c->item_size; + + m->containers[m->n_containers++] = (struct bus_container) { + .enclosing = type, + .signature = TAKE_PTR(signature), - w->array_size = array_size; - w->item_size = item_size; - w->offsets = TAKE_PTR(offsets); - w->n_offsets = n_offsets; - w->offset_index = 0; + .before = before, + .begin = m->rindex, + /* Unary type has fixed size of 1, but virtual size of 0 */ + .end = end, + .array_size = array_size, + .item_size = item_size, + .offsets = TAKE_PTR(offsets), + .n_offsets = n_offsets, + }; return 1; } From 6d1e0f4fcba8d6f425da3dc91805db95399b3c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 9 Jul 2018 08:06:28 +0200 Subject: [PATCH 11/28] sd-bus: unify three code-paths which free struct bus_container We didn't free one of the fields in two of the places. $ valgrind --show-leak-kinds=all --leak-check=full \ build/fuzz-bus-message \ test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 ... ==14457== HEAP SUMMARY: ==14457== in use at exit: 3 bytes in 1 blocks ==14457== total heap usage: 509 allocs, 508 frees, 51,016 bytes allocated ==14457== ==14457== 3 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==14457== at 0x4C2EBAB: malloc (vg_replace_malloc.c:299) ==14457== by 0x53AFE79: strndup (in /usr/lib64/libc-2.27.so) ==14457== by 0x4F52EB8: free_and_strndup (string-util.c:1039) ==14457== by 0x4F8E1AB: sd_bus_message_peek_type (bus-message.c:4193) ==14457== by 0x4F76CB5: bus_message_dump (bus-dump.c:144) ==14457== by 0x108F12: LLVMFuzzerTestOneInput (fuzz-bus-message.c:24) ==14457== by 0x1090F7: main (fuzz-main.c:34) ==14457== ==14457== LEAK SUMMARY: ==14457== definitely lost: 3 bytes in 1 blocks --- src/libsystemd/sd-bus/bus-message.c | 66 +++++++++--------- ...k-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 | Bin 0 -> 534 bytes 2 files changed, 33 insertions(+), 33 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index efbd3307c1..019c8a5f72 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -75,19 +75,38 @@ static void message_reset_parts(sd_bus_message *m) { m->cached_rindex_part_begin = 0; } -static void message_reset_containers(sd_bus_message *m) { - unsigned i; - +static struct bus_container *message_get_container(sd_bus_message *m) { assert(m); - for (i = 0; i < m->n_containers; i++) { - free(m->containers[i].signature); - free(m->containers[i].offsets); - } + if (m->n_containers == 0) + return &m->root_container; + + assert(m->containers); + return m->containers + m->n_containers - 1; +} + +static void message_free_last_container(sd_bus_message *m) { + struct bus_container *c; + + c = message_get_container(m); + + free(c->signature); + free(c->peeked_signature); + free(c->offsets); + + /* Move to previous container, but not if we are on root container */ + if (m->n_containers > 0) + m->n_containers--; +} + +static void message_reset_containers(sd_bus_message *m) { + assert(m); + + while (m->n_containers > 0) + message_free_last_container(m); m->containers = mfree(m->containers); - - m->n_containers = m->containers_allocated = 0; + m->containers_allocated = 0; m->root_container.index = 0; } @@ -110,10 +129,8 @@ static sd_bus_message* message_free(sd_bus_message *m) { free(m->iovec); message_reset_containers(m); - free(m->root_container.signature); - free(m->root_container.offsets); - - free(m->root_container.peeked_signature); + assert(m->n_containers == 0); + message_free_last_container(m); bus_creds_done(&m->creds); return mfree(m); @@ -1088,16 +1105,6 @@ _public_ int sd_bus_message_set_allow_interactive_authorization(sd_bus_message * return 0; } -static struct bus_container *message_get_container(sd_bus_message *m) { - assert(m); - - if (m->n_containers == 0) - return &m->root_container; - - assert(m->containers); - return m->containers + m->n_containers - 1; -} - struct bus_body_part *message_append_part(sd_bus_message *m) { struct bus_body_part *part; @@ -4073,13 +4080,9 @@ _public_ int sd_bus_message_exit_container(sd_bus_message *m) { return -EBUSY; } - free(c->signature); - free(c->peeked_signature); - free(c->offsets); - m->n_containers--; + message_free_last_container(m); c = message_get_container(m); - saved = c->index; c->index = c->saved_index; r = container_next_item(m, c, &m->rindex); @@ -4097,16 +4100,13 @@ static void message_quit_container(sd_bus_message *m) { assert(m->sealed); assert(m->n_containers > 0); - c = message_get_container(m); - /* Undo seeks */ + c = message_get_container(m); assert(m->rindex >= c->before); m->rindex = c->before; /* Free container */ - free(c->signature); - free(c->offsets); - m->n_containers--; + message_free_last_container(m); /* Correct index of new top-level container */ c = message_get_container(m); diff --git a/test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 b/test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20 new file mode 100644 index 0000000000000000000000000000000000000000..c371824ffb604708619fd0713e8fca609bac18f7 GIT binary patch literal 534 zcmZ{h!A`?442GSJP20o?A&zJgm*%pT#&`l!4rxq{&>8YmwQrOs;B(}I_m11m8`nFp`#ek1>oQYVSs`!XH?7Y=}3y9Ye+UliL9^x9s66$8wH+TPdOG`n| z5Uhx Date: Mon, 9 Jul 2018 10:52:51 +0200 Subject: [PATCH 12/28] bus-message: avoid an infinite loop on empty structures The alternative would be to treat gvariant and !gvariant messages differently. But this is a problem because we check signatures is variuos places before we have an actual message, for example in sd_bus_add_object_vtable(). It seems better to treat things consistent (i.e. follow the lowest common denominator) and disallow empty structures everywhere. --- src/libsystemd/sd-bus/bus-message.c | 3 ++- src/libsystemd/sd-bus/bus-signature.c | 6 ++++++ src/libsystemd/sd-bus/test-bus-gvariant.c | 6 +++--- src/libsystemd/sd-bus/test-bus-marshal.c | 6 +++--- src/libsystemd/sd-bus/test-bus-signature.c | 8 ++++---- ...crash-26bba7182dedc8848939931d9fcefcb7922f2e56 | Bin 0 -> 157 bytes ...meout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b | Bin 0 -> 534 bytes 7 files changed, 18 insertions(+), 11 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56 create mode 100644 test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 019c8a5f72..80d4407f0b 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4147,6 +4147,7 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char /* signature_element_length does verification internally */ + /* The array element must not be empty */ assert(l >= 1); if (free_and_strndup(&c->peeked_signature, c->signature + c->index + 1, l) < 0) @@ -4170,7 +4171,7 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (r < 0) return r; - assert(l >= 2); + assert(l >= 3); if (free_and_strndup(&c->peeked_signature, c->signature + c->index + 1, l - 2) < 0) return -ENOMEM; diff --git a/src/libsystemd/sd-bus/bus-signature.c b/src/libsystemd/sd-bus/bus-signature.c index 06ef1ee4cb..1ecd6e8b7e 100644 --- a/src/libsystemd/sd-bus/bus-signature.c +++ b/src/libsystemd/sd-bus/bus-signature.c @@ -56,6 +56,12 @@ static int signature_element_length_internal( p += t; } + if (p - s < 2) + /* D-Bus spec: Empty structures are not allowed; there + * must be at least one type code between the parentheses. + */ + return -EINVAL; + *l = p - s + 1; return 0; } diff --git a/src/libsystemd/sd-bus/test-bus-gvariant.c b/src/libsystemd/sd-bus/test-bus-gvariant.c index 92324bff29..8a4ad0ee72 100644 --- a/src/libsystemd/sd-bus/test-bus-gvariant.c +++ b/src/libsystemd/sd-bus/test-bus-gvariant.c @@ -18,7 +18,7 @@ static void test_bus_gvariant_is_fixed_size(void) { assert_se(bus_gvariant_is_fixed_size("") > 0); - assert_se(bus_gvariant_is_fixed_size("()") > 0); + assert_se(bus_gvariant_is_fixed_size("()") == -EINVAL); assert_se(bus_gvariant_is_fixed_size("y") > 0); assert_se(bus_gvariant_is_fixed_size("u") > 0); assert_se(bus_gvariant_is_fixed_size("b") > 0); @@ -43,7 +43,7 @@ static void test_bus_gvariant_is_fixed_size(void) { static void test_bus_gvariant_get_size(void) { assert_se(bus_gvariant_get_size("") == 0); - assert_se(bus_gvariant_get_size("()") == 1); + assert_se(bus_gvariant_get_size("()") == -EINVAL); assert_se(bus_gvariant_get_size("y") == 1); assert_se(bus_gvariant_get_size("u") == 4); assert_se(bus_gvariant_get_size("b") == 1); @@ -75,7 +75,7 @@ static void test_bus_gvariant_get_size(void) { static void test_bus_gvariant_get_alignment(void) { assert_se(bus_gvariant_get_alignment("") == 1); - assert_se(bus_gvariant_get_alignment("()") == 1); + assert_se(bus_gvariant_get_alignment("()") == -EINVAL); assert_se(bus_gvariant_get_alignment("y") == 1); assert_se(bus_gvariant_get_alignment("b") == 1); assert_se(bus_gvariant_get_alignment("u") == 4); diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 9e09b8f61c..d1c674b223 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/src/libsystemd/sd-bus/test-bus-marshal.c @@ -154,7 +154,7 @@ int main(int argc, char *argv[]) { assert_se(r >= 0); r = sd_bus_message_append(m, "()"); - assert_se(r >= 0); + assert_se(r == -EINVAL); r = sd_bus_message_append(m, "ba(ss)", 255, 3, "aaa", "1", "bbb", "2", "ccc", "3"); assert_se(r >= 0); @@ -296,7 +296,7 @@ int main(int argc, char *argv[]) { assert_se(v == 10); r = sd_bus_message_read(m, "()"); - assert_se(r > 0); + assert_se(r < 0); r = sd_bus_message_read(m, "ba(ss)", &boolean, 3, &x, &y, &a, &b, &c, &d); assert_se(r > 0); @@ -377,7 +377,7 @@ int main(int argc, char *argv[]) { assert_se(sd_bus_message_verify_type(m, 'a', "{yv}") > 0); - r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y()"); + r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y"); assert_se(r >= 0); assert_se(sd_bus_message_verify_type(m, 'b', NULL) > 0); diff --git a/src/libsystemd/sd-bus/test-bus-signature.c b/src/libsystemd/sd-bus/test-bus-signature.c index ab59d04e20..84648dbc2a 100644 --- a/src/libsystemd/sd-bus/test-bus-signature.c +++ b/src/libsystemd/sd-bus/test-bus-signature.c @@ -14,9 +14,9 @@ int main(int argc, char *argv[]) { assert_se(signature_is_single("v", false)); assert_se(signature_is_single("as", false)); assert_se(signature_is_single("(ss)", false)); - assert_se(signature_is_single("()", false)); - assert_se(signature_is_single("(()()()()())", false)); - assert_se(signature_is_single("(((())))", false)); + assert_se(!signature_is_single("()", false)); + assert_se(!signature_is_single("(()()()()())", false)); + assert_se(!signature_is_single("(((())))", false)); assert_se(signature_is_single("((((s))))", false)); assert_se(signature_is_single("{ss}", true)); assert_se(signature_is_single("a{ss}", false)); @@ -61,7 +61,7 @@ int main(int argc, char *argv[]) { assert_se(signature_is_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaas", false)); assert_se(!signature_is_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaau", false)); - assert_se(signature_is_valid("(((((((((((((((((((((((((((((((())))))))))))))))))))))))))))))))", false)); + assert_se(signature_is_valid("((((((((((((((((((((((((((((((((s))))))))))))))))))))))))))))))))", false)); assert_se(!signature_is_valid("((((((((((((((((((((((((((((((((()))))))))))))))))))))))))))))))))", false)); assert_se(namespace_complex_pattern("", "")); diff --git a/test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56 b/test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56 new file mode 100644 index 0000000000000000000000000000000000000000..f1bf3229effc982c8b129182fe60739efe3c5013 GIT binary patch literal 157 mcmd1#|DTC5gMmSS0SHWtK_neOii>$FgGM4Akdcw0DF6TjSP;el literal 0 HcmV?d00001 diff --git a/test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b b/test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b new file mode 100644 index 0000000000000000000000000000000000000000..c975f906eef521a3cfac5627c8b371ee55aa0e6c GIT binary patch literal 534 zcmcJL!Ab-%42J(Y?m8o$d;nSS(q4Ae_Yi!A47)oFEOwaGU5e<<_x8`!K@h}~fslMn zn)c7Z!M!`6y9Pc0I2S?0hHh3l#W~|szZ;Ct$XAT}7+V?FCpm1RoiBemuU&_Ys%S@7 zdCkYS>{AZe=OjL~Ie67zrCwgdYud(O^J==RG>!dpXFS^tlZIX@tK0h@{D4MV@hJsW zyR)R1zXEs6tHM*H04&I}2-7)y>9oEVCxw(Vn{LBxXmJ)=frMcRdZlJ-~v b#4gh=OPF@NW^U~wGO=l?@b9nvy Date: Mon, 9 Jul 2018 11:12:33 +0200 Subject: [PATCH 13/28] bus-message: let's always use -EBADMSG when the message is bad -EINVAL means the arguments were somehow wrong, so translate the code we get internally into -EBADMSG when returning. --- src/libsystemd/sd-bus/bus-message.c | 2 ++ .../crash-c1b37b4729b42c0c05b23cba4eed5d8102498a1e | Bin 0 -> 93 bytes 2 files changed, 2 insertions(+) create mode 100644 test/fuzz/fuzz-bus-message/crash-c1b37b4729b42c0c05b23cba4eed5d8102498a1e diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 80d4407f0b..41760b5915 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5385,6 +5385,8 @@ int bus_message_parse_fields(sd_bus_message *m) { &m->root_container.item_size, &m->root_container.offsets, &m->root_container.n_offsets); + if (r == -EINVAL) + return -EBADMSG; if (r < 0) return r; } diff --git a/test/fuzz/fuzz-bus-message/crash-c1b37b4729b42c0c05b23cba4eed5d8102498a1e b/test/fuzz/fuzz-bus-message/crash-c1b37b4729b42c0c05b23cba4eed5d8102498a1e new file mode 100644 index 0000000000000000000000000000000000000000..2ae1a8715a12c65fba27d8e60216112a99b0ace7 GIT binary patch literal 93 wcmd1FDP>|PH8L_f3B<@i03SeB2xg~!`?q0o*WZ8t85 Date: Mon, 9 Jul 2018 13:21:44 +0200 Subject: [PATCH 14/28] bus-message: do not crash on message with a string of zero length We'd calculate the "real" length of the string as 'item_size - 1', which does not work out well when item_size == 0. --- src/libsystemd/sd-bus/bus-message.c | 6 ++++++ .../crash-29ed3c202e0ffade3cad42c8bbeb6cc68a21eb8e | Bin 0 -> 51 bytes 2 files changed, 6 insertions(+) create mode 100644 test/fuzz/fuzz-bus-message/crash-29ed3c202e0ffade3cad42c8bbeb6cc68a21eb8e diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 41760b5915..76df43e095 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3292,6 +3292,12 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m, char type, void *p) { if (IN_SET(type, SD_BUS_TYPE_STRING, SD_BUS_TYPE_OBJECT_PATH, SD_BUS_TYPE_SIGNATURE)) { bool ok; + /* D-Bus spec: The marshalling formats for the string-like types all end + * with a single zero (NUL) byte, but that byte is not considered to be part + * of the text. */ + if (c->item_size == 0) + return -EBADMSG; + r = message_peek_body(m, &rindex, 1, c->item_size, &q); if (r < 0) return r; diff --git a/test/fuzz/fuzz-bus-message/crash-29ed3c202e0ffade3cad42c8bbeb6cc68a21eb8e b/test/fuzz/fuzz-bus-message/crash-29ed3c202e0ffade3cad42c8bbeb6cc68a21eb8e new file mode 100644 index 0000000000000000000000000000000000000000..4488f0a6c685b5d43eddbe41a0c6a3b6be9b02e2 GIT binary patch literal 51 fcmc~1WMC4sJpJnr13KV`0|t%6q+%$@&=ddw)CUPg literal 0 HcmV?d00001 From 9c65778d614588d21645163dea97a5fe2c1c4ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Jul 2018 20:14:39 +0200 Subject: [PATCH 15/28] bus-message: rename function for clarity There's already message_free_last_container(), so rename to match. --- src/libsystemd/sd-bus/bus-message.c | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 76df43e095..8e970b1231 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -75,7 +75,7 @@ static void message_reset_parts(sd_bus_message *m) { m->cached_rindex_part_begin = 0; } -static struct bus_container *message_get_container(sd_bus_message *m) { +static struct bus_container *message_get_last_container(sd_bus_message *m) { assert(m); if (m->n_containers == 0) @@ -88,7 +88,7 @@ static struct bus_container *message_get_container(sd_bus_message *m) { static void message_free_last_container(sd_bus_message *m) { struct bus_container *c; - c = message_get_container(m); + c = message_get_last_container(m); free(c->signature); free(c->peeked_signature); @@ -1195,7 +1195,7 @@ static int message_add_offset(sd_bus_message *m, size_t offset) { /* Add offset to current container, unless this is the first * item in it, which will have the 0 offset, which we can * ignore. */ - c = message_get_container(m); + c = message_get_last_container(m); if (!c->need_offsets) return 0; @@ -1367,7 +1367,7 @@ int message_append_basic(sd_bus_message *m, char type, const void *p, const void assert_return(bus_type_is_basic(type), -EINVAL); assert_return(!m->poisoned, -ESTALE); - c = message_get_container(m); + c = message_get_last_container(m); if (c->signature && c->signature[c->index]) { /* Container signature is already set */ @@ -1560,7 +1560,7 @@ _public_ int sd_bus_message_append_string_space( assert_return(!m->sealed, -EPERM); assert_return(!m->poisoned, -ESTALE); - c = message_get_container(m); + c = message_get_last_container(m); if (c->signature && c->signature[c->index]) { /* Container signature is already set */ @@ -1949,7 +1949,7 @@ _public_ int sd_bus_message_open_container( return -ENOMEM; } - c = message_get_container(m); + c = message_get_last_container(m); signature = strdup(contents); if (!signature) { @@ -2174,7 +2174,7 @@ _public_ int sd_bus_message_close_container(sd_bus_message *m) { assert_return(m->n_containers > 0, -EINVAL); assert_return(!m->poisoned, -ESTALE); - c = message_get_container(m); + c = message_get_last_container(m); if (c->enclosing != SD_BUS_TYPE_ARRAY) if (c->signature && c->signature[c->index] != 0) @@ -2678,7 +2678,7 @@ _public_ int sd_bus_message_append_string_memfd( if (size > (uint64_t) (uint32_t) -1) return -EINVAL; - c = message_get_container(m); + c = message_get_last_container(m); if (c->signature && c->signature[c->index]) { /* Container signature is already set */ @@ -3011,7 +3011,7 @@ static bool message_end_of_signature(sd_bus_message *m) { assert(m); - c = message_get_container(m); + c = message_get_last_container(m); return !c->signature || c->signature[c->index] == 0; } @@ -3020,7 +3020,7 @@ static bool message_end_of_array(sd_bus_message *m, size_t index) { assert(m); - c = message_get_container(m); + c = message_get_last_container(m); if (c->enclosing != SD_BUS_TYPE_ARRAY) return false; @@ -3281,7 +3281,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m, char type, void *p) { if (message_end_of_array(m, m->rindex)) return 0; - c = message_get_container(m); + c = message_get_last_container(m); if (c->signature[c->index] != type) return -ENXIO; @@ -4011,7 +4011,7 @@ _public_ int sd_bus_message_enter_container(sd_bus_message *m, if (message_end_of_array(m, m->rindex)) return 0; - c = message_get_container(m); + c = message_get_last_container(m); signature = strdup(contents); if (!signature) @@ -4067,7 +4067,7 @@ _public_ int sd_bus_message_exit_container(sd_bus_message *m) { assert_return(m->sealed, -EPERM); assert_return(m->n_containers > 0, -ENXIO); - c = message_get_container(m); + c = message_get_last_container(m); if (c->enclosing != SD_BUS_TYPE_ARRAY) { if (c->signature && c->signature[c->index] != 0) @@ -4088,7 +4088,7 @@ _public_ int sd_bus_message_exit_container(sd_bus_message *m) { message_free_last_container(m); - c = message_get_container(m); + c = message_get_last_container(m); saved = c->index; c->index = c->saved_index; r = container_next_item(m, c, &m->rindex); @@ -4107,7 +4107,7 @@ static void message_quit_container(sd_bus_message *m) { assert(m->n_containers > 0); /* Undo seeks */ - c = message_get_container(m); + c = message_get_last_container(m); assert(m->rindex >= c->before); m->rindex = c->before; @@ -4115,7 +4115,7 @@ static void message_quit_container(sd_bus_message *m) { message_free_last_container(m); /* Correct index of new top-level container */ - c = message_get_container(m); + c = message_get_last_container(m); c->index = c->saved_index; } @@ -4132,7 +4132,7 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (message_end_of_array(m, m->rindex)) goto eof; - c = message_get_container(m); + c = message_get_last_container(m); if (bus_type_is_basic(c->signature[c->index])) { if (contents) @@ -4276,9 +4276,9 @@ _public_ int sd_bus_message_rewind(sd_bus_message *m, int complete) { message_reset_containers(m); m->rindex = 0; - c = message_get_container(m); + c = message_get_last_container(m); } else { - c = message_get_container(m); + c = message_get_last_container(m); c->offset_index = 0; c->index = 0; @@ -4523,7 +4523,7 @@ _public_ int sd_bus_message_skip(sd_bus_message *m, const char *types) { if (message_end_of_array(m, m->rindex)) return 0; - c = message_get_container(m); + c = message_get_last_container(m); r = signature_element_length(c->signature + c->index, &l); if (r < 0) @@ -4689,7 +4689,7 @@ _public_ int sd_bus_message_read_array( if (r <= 0) return r; - c = message_get_container(m); + c = message_get_last_container(m); if (BUS_MESSAGE_IS_GVARIANT(m)) { align = bus_gvariant_get_alignment(CHAR_TO_STR(type)); @@ -5588,7 +5588,7 @@ _public_ const char* sd_bus_message_get_signature(sd_bus_message *m, int complet assert_return(m, NULL); - c = complete ? &m->root_container : message_get_container(m); + c = complete ? &m->root_container : message_get_last_container(m); return strempty(c->signature); } From f22c308aff556bf5c6599ffcb61e637e366ab232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Jul 2018 21:24:53 +0200 Subject: [PATCH 16/28] bus-message: use define --- src/libsystemd/sd-bus/bus-message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 8e970b1231..434a282490 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -225,7 +225,7 @@ static int message_append_field_string( /* dbus1 doesn't allow strings over 32bit, let's enforce this * globally, to not risk convertability */ l = strlen(s); - if (l > (size_t) (uint32_t) -1) + if (l > UINT32_MAX) return -EINVAL; /* Signature "(yv)" where the variant contains "s" */ From e8fd7e4b5b5269377efc641a7da43850822c1250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 2 Aug 2018 00:46:20 +0200 Subject: [PATCH 17/28] bus: do not print (null) if the message has unknown type --- src/libsystemd/sd-bus/bus-dump.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-dump.c b/src/libsystemd/sd-bus/bus-dump.c index 888f161768..506ed0d73c 100644 --- a/src/libsystemd/sd-bus/bus-dump.c +++ b/src/libsystemd/sd-bus/bus-dump.c @@ -57,8 +57,14 @@ int bus_message_dump(sd_bus_message *m, FILE *f, unsigned flags) { "%s%s%s Type=%s%s%s Endian=%c Flags=%u Version=%u Priority=%"PRIi64, m->header->type == SD_BUS_MESSAGE_METHOD_ERROR ? ansi_highlight_red() : m->header->type == SD_BUS_MESSAGE_METHOD_RETURN ? ansi_highlight_green() : - m->header->type != SD_BUS_MESSAGE_SIGNAL ? ansi_highlight() : "", special_glyph(TRIANGULAR_BULLET), ansi_normal(), - ansi_highlight(), bus_message_type_to_string(m->header->type), ansi_normal(), + m->header->type != SD_BUS_MESSAGE_SIGNAL ? ansi_highlight() : "", + special_glyph(TRIANGULAR_BULLET), + ansi_normal(), + + ansi_highlight(), + bus_message_type_to_string(m->header->type) ?: "(unknown)", + ansi_normal(), + m->header->endian, m->header->flags, m->header->version, From 12603b84d2fb07603e2ea94b240c6b78ad17510e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 2 Aug 2018 14:25:11 +0200 Subject: [PATCH 18/28] bus-message: fix calculation of offsets table The offsets specify the ends of variable length data. We would trust the incoming data, putting the offsets specified in our message into the offsets tables after doing some superficial verification. But when actually reading the data we apply alignment, so we would take the previous offset, align it, making it bigger then current offset, and then we'd try to read data of negative length. In the attached example, the message specifies the following offsets: [1, 4] but the alignment of those items is [1, 8] so we'd calculate the second item as starting at 8 and ending at 4. --- src/libsystemd/sd-bus/bus-message.c | 36 +++++++++--------- ...h-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 | Bin 0 -> 28 bytes 2 files changed, 18 insertions(+), 18 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 434a282490..22cb38d9c5 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3115,6 +3115,7 @@ static int container_next_item(sd_bus_message *m, struct bus_container *c, size_ assert(alignment > 0); *rindex = ALIGN_TO(c->offsets[c->offset_index], alignment); + assert(c->offsets[c->offset_index+1] >= *rindex); c->item_size = c->offsets[c->offset_index+1] - *rindex; } else { @@ -3154,6 +3155,7 @@ static int container_next_item(sd_bus_message *m, struct bus_container *c, size_ assert(alignment > 0); *rindex = ALIGN_TO(c->offsets[c->offset_index], alignment); + assert(c->offsets[c->offset_index+1] >= *rindex); c->item_size = c->offsets[c->offset_index+1] - *rindex; c->offset_index++; @@ -3700,7 +3702,7 @@ static int build_struct_offsets( size_t *n_offsets) { unsigned n_variable = 0, n_total = 0, v; - size_t previous = 0, where; + size_t previous, where; const char *p; size_t sz; void *q; @@ -3779,6 +3781,7 @@ static int build_struct_offsets( /* Second, loop again and build an offset table */ p = signature; + previous = m->rindex; while (*p != 0) { size_t n, offset; int k; @@ -3792,37 +3795,34 @@ static int build_struct_offsets( memcpy(t, p, n); t[n] = 0; + size_t align = bus_gvariant_get_alignment(t); + assert(align > 0); + + /* The possible start of this member after including alignment */ + size_t start = ALIGN_TO(previous, align); + k = bus_gvariant_get_size(t); if (k < 0) { size_t x; - /* variable size */ + /* Variable size */ if (v > 0) { v--; x = bus_gvariant_read_word_le((uint8_t*) q + v*sz, sz); if (x >= size) return -EBADMSG; - if (m->rindex + x < previous) - return -EBADMSG; } else - /* The last item's end - * is determined from - * the start of the - * offset array */ + /* The last item's end is determined + * from the start of the offset array */ x = size - (n_variable * sz); offset = m->rindex + x; - - } else { - size_t align; - - /* fixed size */ - align = bus_gvariant_get_alignment(t); - assert(align > 0); - - offset = (*n_offsets == 0 ? m->rindex : ALIGN_TO((*offsets)[*n_offsets-1], align)) + k; - } + if (offset < start) + return -EBADMSG; + } else + /* Fixed size */ + offset = start + k; } previous = (*offsets)[(*n_offsets)++] = offset; diff --git a/test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 b/test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 new file mode 100644 index 0000000000000000000000000000000000000000..9d3fa0035fd360a37833e8b58cc4aea90df9de83 GIT binary patch literal 28 fcmd1#|DTDG0Z1?a!8`>PAeqj{pplqVrYQgbfcytC literal 0 HcmV?d00001 From 4d82a8d5052fce8c1ea51f8bdec3476fb8cc4747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 2 Aug 2018 14:25:31 +0200 Subject: [PATCH 19/28] bus-message: remove duplicate assignment --- src/libsystemd/sd-bus/bus-message.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 22cb38d9c5..11b050a24d 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4280,7 +4280,6 @@ _public_ int sd_bus_message_rewind(sd_bus_message *m, int complete) { } else { c = message_get_last_container(m); - c->offset_index = 0; c->index = 0; m->rindex = c->begin; } From f88214cf9d66c93f4d22c4c8980de9ee3ff45bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Aug 2018 14:46:57 +0200 Subject: [PATCH 20/28] bus-message: fix calculation of offsets table for arrays This is similar to the grandparent commit 'fix calculation of offsets table', except that now the change is for array elements. Same story as before: we need to make sure that the offsets increase enough taking alignment into account. While at it, rename 'p' to 'previous' to match similar code in other places. --- src/libsystemd/sd-bus/bus-message.c | 17 ++++++++++++----- ...sh-d8f3941c74219b4c03532c9b244d5ea539c61af5 | Bin 0 -> 41 bytes 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-d8f3941c74219b4c03532c9b244d5ea539c61af5 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 11b050a24d..f544ec13ce 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3507,7 +3507,7 @@ static int bus_message_enter_array( size_t rindex; void *q; - int r, alignment; + int r; assert(m); assert(c); @@ -3533,6 +3533,7 @@ static int bus_message_enter_array( if (!BUS_MESSAGE_IS_GVARIANT(m)) { /* dbus1 */ + int alignment; r = message_peek_body(m, &rindex, 4, 4, &q); if (r < 0) @@ -3566,7 +3567,8 @@ static int bus_message_enter_array( *n_offsets = 0; } else { - size_t where, p = 0, framing, sz; + size_t where, previous = 0, framing, sz; + int alignment; unsigned i; /* gvariant: variable length array */ @@ -3594,17 +3596,22 @@ static int bus_message_enter_array( if (!*offsets) return -ENOMEM; + alignment = bus_gvariant_get_alignment(c->signature); + assert(alignment > 0); + for (i = 0; i < *n_offsets; i++) { - size_t x; + size_t x, start; + + start = ALIGN_TO(previous, alignment); x = bus_gvariant_read_word_le((uint8_t*) q + i * sz, sz); if (x > c->item_size - sz) return -EBADMSG; - if (x < p) + if (x < start) return -EBADMSG; (*offsets)[i] = rindex + x; - p = x; + previous = x; } *item_size = (*offsets)[0] - rindex; diff --git a/test/fuzz/fuzz-bus-message/crash-d8f3941c74219b4c03532c9b244d5ea539c61af5 b/test/fuzz/fuzz-bus-message/crash-d8f3941c74219b4c03532c9b244d5ea539c61af5 new file mode 100644 index 0000000000000000000000000000000000000000..26262e1149825a114a89bf9cee5aeca0be463984 GIT binary patch literal 41 rcmd1#|DTC5gMmSS0SHWtIT#p03 Date: Fri, 3 Aug 2018 16:36:51 +0200 Subject: [PATCH 21/28] bus-message: drop asserts in functions which are wrappers for varargs version The function does no processing on it's own, and just forwards arguments to the other function. Let's just use the asserts there. --- src/libsystemd/sd-bus/bus-message.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index f544ec13ce..eee8e39195 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -2444,11 +2444,6 @@ _public_ int sd_bus_message_append(sd_bus_message *m, const char *types, ...) { va_list ap; int r; - assert_return(m, -EINVAL); - assert_return(types, -EINVAL); - assert_return(!m->sealed, -EPERM); - assert_return(!m->poisoned, -ESTALE); - va_start(ap, types); r = sd_bus_message_appendv(m, types, ap); va_end(ap); @@ -4501,10 +4496,6 @@ _public_ int sd_bus_message_read(sd_bus_message *m, const char *types, ...) { va_list ap; int r; - assert_return(m, -EINVAL); - assert_return(m->sealed, -EPERM); - assert_return(types, -EINVAL); - va_start(ap, types); r = sd_bus_message_readv(m, types, ap); va_end(ap); From 10a7ec96d8be4283fb104f4238865328a78e0039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Aug 2018 18:04:02 +0200 Subject: [PATCH 22/28] test-bus-gvariant: turn on debug output I thought the test was wrong, but it turns out one of my patches was at fault. But this helps to diagnose issues. --- src/libsystemd/sd-bus/test-bus-gvariant.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/test-bus-gvariant.c b/src/libsystemd/sd-bus/test-bus-gvariant.c index 8a4ad0ee72..1a9a35d56b 100644 --- a/src/libsystemd/sd-bus/test-bus-gvariant.c +++ b/src/libsystemd/sd-bus/test-bus-gvariant.c @@ -17,6 +17,8 @@ #include "util.h" static void test_bus_gvariant_is_fixed_size(void) { + log_info("/* %s */", __func__); + assert_se(bus_gvariant_is_fixed_size("") > 0); assert_se(bus_gvariant_is_fixed_size("()") == -EINVAL); assert_se(bus_gvariant_is_fixed_size("y") > 0); @@ -42,6 +44,8 @@ static void test_bus_gvariant_is_fixed_size(void) { } static void test_bus_gvariant_get_size(void) { + log_info("/* %s */", __func__); + assert_se(bus_gvariant_get_size("") == 0); assert_se(bus_gvariant_get_size("()") == -EINVAL); assert_se(bus_gvariant_get_size("y") == 1); @@ -74,6 +78,8 @@ static void test_bus_gvariant_get_size(void) { } static void test_bus_gvariant_get_alignment(void) { + log_info("/* %s */", __func__); + assert_se(bus_gvariant_get_alignment("") == 1); assert_se(bus_gvariant_get_alignment("()") == -EINVAL); assert_se(bus_gvariant_get_alignment("y") == 1); @@ -129,7 +135,10 @@ static int test_marshal(void) { bus->message_version = 2; /* dirty hack to enable gvariant */ - assert_se(sd_bus_message_new_method_call(bus, &m, "a.service.name", "/an/object/path/which/is/really/really/long/so/that/we/hit/the/eight/bit/boundary/by/quite/some/margin/to/test/this/stuff/that/it/really/works", "an.interface.name", "AMethodName") >= 0); + r = sd_bus_message_new_method_call(bus, &m, "a.service.name", + "/an/object/path/which/is/really/really/long/so/that/we/hit/the/eight/bit/boundary/by/quite/some/margin/to/test/this/stuff/that/it/really/works", + "an.interface.name", "AMethodName"); + assert_se(r >= 0); assert_cc(sizeof(struct bus_header) == 16); @@ -202,7 +211,7 @@ static int test_marshal(void) { } int main(int argc, char *argv[]) { - test_setup_logging(LOG_INFO); + test_setup_logging(LOG_DEBUG); test_bus_gvariant_is_fixed_size(); test_bus_gvariant_get_size(); From 0b4775b52747bebf7ecb62062798475629767044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Aug 2018 18:05:27 +0200 Subject: [PATCH 23/28] bus-message: output debug information about offset troubles --- src/libsystemd/sd-bus/bus-message.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index eee8e39195..7fb48cb330 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3820,8 +3820,11 @@ static int build_struct_offsets( x = size - (n_variable * sz); offset = m->rindex + x; - if (offset < start) + if (offset < start) { + log_debug("For type %s with alignment %zu, message specifies offset %zu which is smaller than previous end %zu + alignment = %zu", + t, align, offset, previous, start); return -EBADMSG; + } } else /* Fixed size */ offset = start + k; From 73777ddba5100fe6c0791cd37a91f24a515f3202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Aug 2018 08:32:20 +0200 Subject: [PATCH 24/28] bus-message: fix skipping of array fields in !gvariant messages We copied part of the string into a buffer that was off by two. If the element signature had length one, we'd copy 0 bytes and crash when looking at the "first" byte. Otherwise, we would crash because strncpy would not terminate the string. --- src/libsystemd/sd-bus/bus-message.c | 8 ++++---- ...crash-37449529b1ad867f0c2671fa80aca5d7812a2b70 | Bin 0 -> 534 bytes 2 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-37449529b1ad867f0c2671fa80aca5d7812a2b70 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 7fb48cb330..b1d89fddc4 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4958,18 +4958,18 @@ static int message_skip_fields( } else if (t == SD_BUS_TYPE_ARRAY) { - r = signature_element_length(*signature+1, &l); + r = signature_element_length(*signature + 1, &l); if (r < 0) return r; assert(l >= 1); { - char sig[l-1], *s; + char sig[l + 1], *s = sig; uint32_t nas; int alignment; - strncpy(sig, *signature + 1, l-1); - s = sig; + strncpy(sig, *signature + 1, l); + sig[l] = '\0'; alignment = bus_type_get_alignment(sig[0]); if (alignment < 0) diff --git a/test/fuzz/fuzz-bus-message/crash-37449529b1ad867f0c2671fa80aca5d7812a2b70 b/test/fuzz/fuzz-bus-message/crash-37449529b1ad867f0c2671fa80aca5d7812a2b70 new file mode 100644 index 0000000000000000000000000000000000000000..6a20265a39e1b4a318b50aee2b13727ddc4113bf GIT binary patch literal 534 zcmc~{WMHggWMD`aVqj=xU|>*W&P&W-;Q0Fg|9>Elfq|V9OfmRED27Bi2!jjC2Wn-| z17hYPAOVtNW-Ml42GVKy`9P9^ffdMS1=8h-IVt%J91NTwNgyEFV4&K>#6$*=MMgl( r%#fH?l1eMv=;=K=_yi-CK!KUB2_%6r0c0u^mlS2@rGxk|0FGY(dwVLU literal 0 HcmV?d00001 From 3d338a302f56c0ef0445660d9856794abe1af8b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Aug 2018 09:02:48 +0200 Subject: [PATCH 25/28] bus-message: also properly copy struct signature when skipping The change is similar to that in the previous commit, but I don't have a reproducer / test case case for this one, so I'm keeping it seperate. --- src/libsystemd/sd-bus/bus-message.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index b1d89fddc4..003adcf134 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5013,9 +5013,9 @@ static int message_skip_fields( assert(l >= 2); { - char sig[l-1], *s; - strncpy(sig, *signature + 1, l-1); - s = sig; + char sig[l + 1], *s = sig; + strncpy(sig, *signature + 1, l); + sig[l] = '\0'; r = message_skip_fields(m, ri, (uint32_t) -1, (const char**) &s); if (r < 0) From edde66ffc2404de58e8b19810951f376efb344da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Aug 2018 11:31:45 +0200 Subject: [PATCH 26/28] fuzz-bus-message: add two test cases that pass now It seems that they got fixed by one of the patches. Let's add them just in case. --- ...crash-32bf69483cbd4f2e6d46c25a2f92a472109aee45 | Bin 0 -> 89 bytes ...crash-4f0211eb269e28db941961061494bfdbf3345e54 | Bin 0 -> 143 bytes 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-32bf69483cbd4f2e6d46c25a2f92a472109aee45 create mode 100644 test/fuzz/fuzz-bus-message/crash-4f0211eb269e28db941961061494bfdbf3345e54 diff --git a/test/fuzz/fuzz-bus-message/crash-32bf69483cbd4f2e6d46c25a2f92a472109aee45 b/test/fuzz/fuzz-bus-message/crash-32bf69483cbd4f2e6d46c25a2f92a472109aee45 new file mode 100644 index 0000000000000000000000000000000000000000..aa0c6ff7f7b6d2e3fa4358716ee1d05ba74cefc0 GIT binary patch literal 89 scmc~q)(CT7OTMiz#q2KEMtDT&4= liD{N8W@+Y0hK!61V4zWvq@j_NlvJFQnL&wm%}Py80RUy_C0PIf literal 0 HcmV?d00001 From d831fb6f2bde829f9309aea242f502587662d1cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 11 Aug 2018 11:43:09 +0200 Subject: [PATCH 27/28] bus-message: return -EBADMSG not -EINVAL on invalid !gvariant messages --- src/libsystemd/sd-bus/bus-message.c | 2 +- ...crash-4162a61a79e4c5a832ca5232212f75fa560a1f75 | Bin 0 -> 534 bytes 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 test/fuzz/fuzz-bus-message/crash-4162a61a79e4c5a832ca5232212f75fa560a1f75 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 003adcf134..a484dc41c4 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -5024,7 +5024,7 @@ static int message_skip_fields( *signature += l; } else - return -EINVAL; + return -EBADMSG; } } diff --git a/test/fuzz/fuzz-bus-message/crash-4162a61a79e4c5a832ca5232212f75fa560a1f75 b/test/fuzz/fuzz-bus-message/crash-4162a61a79e4c5a832ca5232212f75fa560a1f75 new file mode 100644 index 0000000000000000000000000000000000000000..5faf3308e7ac9c14d66422169e74ba8c05ad7319 GIT binary patch literal 534 zcmd5(y$ZrW3{L#Rf|Cy*1sA)t;uE+zxcCZJw53qIqj#v2xH$UGez{(yI63-3NWO$5 zU+!uqzB5rdCwdYQvnEi=V1glA8o?i`lMy}upTQSe=c-Assy=GTr+lHv=4$0!Vy$EX z_LzYX&1*Ob(W(=vPGKsxuBpzYaDn6&un5*x;uk`Xz?Yk^O%qgGJ(zd Date: Thu, 23 Aug 2018 14:48:40 +0200 Subject: [PATCH 28/28] bus-message: avoid wrap-around when using length read from message We would read (-1), and then add 1 to it, call message_peek_body(..., 0, ...), and when trying to make use of the data. The fuzzer test case is just for one site, but they all look similar. v2: fix two UINT8_MAX/UINT32_MAX mismatches founds by LGTM --- src/libsystemd/sd-bus/bus-message.c | 24 ++++++++++++++++++ ...h-603dfd98252375ac7dbced53c2ec312671939a36 | Bin 0 -> 40 bytes 2 files changed, 24 insertions(+) create mode 100644 test/fuzz/fuzz-bus-message/crash-603dfd98252375ac7dbced53c2ec312671939a36 diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index a484dc41c4..697553e74e 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3389,6 +3389,10 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m, char type, void *p) { return r; l = BUS_MESSAGE_BSWAP32(m, *(uint32_t*) q); + if (l == UINT32_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_body(m, &rindex, 1, l+1, &q); if (r < 0) return r; @@ -3411,6 +3415,10 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m, char type, void *p) { return r; l = *(uint8_t*) q; + if (l == UINT8_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_body(m, &rindex, 1, l+1, &q); if (r < 0) return r; @@ -3676,6 +3684,10 @@ static int bus_message_enter_variant( return r; l = *(uint8_t*) q; + if (l == UINT8_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_body(m, &rindex, 1, l+1, &q); if (r < 0) return r; @@ -4244,6 +4256,10 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char return r; l = *(uint8_t*) q; + if (l == UINT8_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_body(m, &rindex, 1, l+1, &q); if (r < 0) return r; @@ -4826,6 +4842,10 @@ static int message_peek_field_string( if (r < 0) return r; + if (l == UINT32_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_fields(m, ri, 1, l+1, &q); if (r < 0) return r; @@ -4877,6 +4897,10 @@ static int message_peek_field_signature( return r; l = *(uint8_t*) q; + if (l == UINT8_MAX) + /* avoid overflow right below */ + return -EBADMSG; + r = message_peek_fields(m, ri, 1, l+1, &q); if (r < 0) return r; diff --git a/test/fuzz/fuzz-bus-message/crash-603dfd98252375ac7dbced53c2ec312671939a36 b/test/fuzz/fuzz-bus-message/crash-603dfd98252375ac7dbced53c2ec312671939a36 new file mode 100644 index 0000000000000000000000000000000000000000..b3fee9e07af4f925697a549bbc8ffc03a277fac0 GIT binary patch literal 40 mcmc~{Vqjzdg7laF|BC@>cE)0c{}2$`*K@IKT2AZ~5ElR}@e}O; literal 0 HcmV?d00001