From 9ccec2afdaf8af463f321eb37d3c3bb90d1d432e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Jun 2022 09:40:45 -0700 Subject: [PATCH 1/2] CVE-2022-32742: s4: torture: Add raw.write.bad-write test. Reproduces the test code in: BUG: https://bugzilla.samba.org/show_bug.cgi?id=15085 Add knownfail. Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp --- selftest/knownfail.d/bad-write | 2 + source4/torture/raw/write.c | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 selftest/knownfail.d/bad-write diff --git a/selftest/knownfail.d/bad-write b/selftest/knownfail.d/bad-write new file mode 100644 index 00000000000..5fc16606a13 --- /dev/null +++ b/selftest/knownfail.d/bad-write @@ -0,0 +1,2 @@ +^samba3.raw.write.bad-write\(nt4_dc_smb1\) +^samba3.raw.write.bad-write\(ad_dc_smb1\) diff --git a/source4/torture/raw/write.c b/source4/torture/raw/write.c index 0a2f50f425b..661485bb548 100644 --- a/source4/torture/raw/write.c +++ b/source4/torture/raw/write.c @@ -25,6 +25,7 @@ #include "libcli/libcli.h" #include "torture/util.h" #include "torture/raw/proto.h" +#include "libcli/raw/raw_proto.h" #define CHECK_STATUS(status, correct) do { \ if (!NT_STATUS_EQUAL(status, correct)) { \ @@ -694,6 +695,93 @@ done: return ret; } +/* + test a deliberately bad SMB1 write. +*/ +static bool test_bad_write(struct torture_context *tctx, + struct smbcli_state *cli) +{ + bool ret = false; + int fnum = -1; + struct smbcli_request *req = NULL; + const char *fname = BASEDIR "\\badwrite.txt"; + bool ok = false; + + if (!torture_setup_dir(cli, BASEDIR)) { + torture_fail(tctx, "failed to setup basedir"); + } + + torture_comment(tctx, "Testing RAW_BAD_WRITE\n"); + + fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); + if (fnum == -1) { + torture_fail_goto(tctx, + done, + talloc_asprintf(tctx, + "Failed to create %s - %s\n", + fname, + smbcli_errstr(cli->tree))); + } + + req = smbcli_request_setup(cli->tree, + SMBwrite, + 5, + 0); + if (req == NULL) { + torture_fail_goto(tctx, + done, + talloc_asprintf(tctx, "talloc fail\n")); + } + + SSVAL(req->out.vwv, VWV(0), fnum); + SSVAL(req->out.vwv, VWV(1), 65535); /* bad write length. */ + SIVAL(req->out.vwv, VWV(2), 0); /* offset */ + SSVAL(req->out.vwv, VWV(4), 0); /* remaining. */ + + if (!smbcli_request_send(req)) { + torture_fail_goto(tctx, + done, + talloc_asprintf(tctx, "Send failed\n")); + } + + if (!smbcli_request_receive(req)) { + torture_fail_goto(tctx, + done, + talloc_asprintf(tctx, "Reveive failed\n")); + } + + /* + * Check for expected error codes. + * ntvfs returns NT_STATUS_UNSUCCESSFUL. + */ + ok = (NT_STATUS_EQUAL(req->status, NT_STATUS_INVALID_PARAMETER) || + NT_STATUS_EQUAL(req->status, NT_STATUS_UNSUCCESSFUL)); + + if (!ok) { + torture_fail_goto(tctx, + done, + talloc_asprintf(tctx, + "Should have returned " + "NT_STATUS_INVALID_PARAMETER or " + "NT_STATUS_UNSUCCESSFUL " + "got %s\n", + nt_errstr(req->status))); + } + + ret = true; + +done: + if (req != NULL) { + smbcli_request_destroy(req); + } + if (fnum != -1) { + smbcli_close(cli->tree, fnum); + } + smb_raw_exit(cli->session); + smbcli_deltree(cli->tree, BASEDIR); + return ret; +} + /* basic testing of write calls */ @@ -705,6 +793,7 @@ struct torture_suite *torture_raw_write(TALLOC_CTX *mem_ctx) torture_suite_add_1smb_test(suite, "write unlock", test_writeunlock); torture_suite_add_1smb_test(suite, "write close", test_writeclose); torture_suite_add_1smb_test(suite, "writex", test_writex); + torture_suite_add_1smb_test(suite, "bad-write", test_bad_write); return suite; } -- 2.34.1 From 9097c5363605e1d5f99ff5a59dc6795c612d472f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 8 Jun 2022 13:50:51 -0700 Subject: [PATCH 2/2] CVE-2022-32742: s3: smbd: Harden the smbreq_bufrem() macro. Fixes the raw.write.bad-write test. NB. We need the two (==0) changes in source3/smbd/reply.c as the gcc optimizer now knows that the return from smbreq_bufrem() can never be less than zero. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15085 Remove knownfail. Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp --- selftest/knownfail.d/bad-write | 2 -- source3/include/smb_macros.h | 2 +- source3/smbd/reply.c | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 selftest/knownfail.d/bad-write diff --git a/selftest/knownfail.d/bad-write b/selftest/knownfail.d/bad-write deleted file mode 100644 index 5fc16606a13..00000000000 --- a/selftest/knownfail.d/bad-write +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.raw.write.bad-write\(nt4_dc_smb1\) -^samba3.raw.write.bad-write\(ad_dc_smb1\) diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h index 344a997cbd2..c75b93fcc25 100644 --- a/source3/include/smb_macros.h +++ b/source3/include/smb_macros.h @@ -152,7 +152,7 @@ /* the remaining number of bytes in smb buffer 'buf' from pointer 'p'. */ #define smb_bufrem(buf, p) (smb_buflen(buf)-PTR_DIFF(p, smb_buf(buf))) -#define smbreq_bufrem(req, p) (req->buflen - PTR_DIFF(p, req->buf)) +#define smbreq_bufrem(req, p) ((req)->buflen < PTR_DIFF((p), (req)->buf) ? 0 : (req)->buflen - PTR_DIFF((p), (req)->buf)) /* Note that chain_size must be available as an extern int to this macro. */ diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index d4573d3da55..e1a47a65662 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -345,7 +345,7 @@ size_t srvstr_get_path_req(TALLOC_CTX *mem_ctx, struct smb_request *req, { ssize_t bufrem = smbreq_bufrem(req, src); - if (bufrem < 0) { + if (bufrem == 0) { *err = NT_STATUS_INVALID_PARAMETER; return 0; } @@ -383,7 +383,7 @@ size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req, { ssize_t bufrem = smbreq_bufrem(req, src); - if (bufrem < 0) { + if (bufrem == 0) { return 0; } -- 2.34.1