[PATCH] Fix panic when running 'smbtorture smb.base'
Volker Lendecke
Volker.Lendecke at SerNet.DE
Tue Apr 9 06:58:13 MDT 2013
On Tue, Apr 09, 2013 at 10:41:05AM +0200, Ralph Wuerthner wrote:
> Hello list,
>
> when running 'smbtorture smb.base' on a recent Samba 4 I noticed the following panic:
>
> [2013/02/23 19:22:58.524698, 0] lib/util.c:810(smb_panic_s3)
> PANIC (pid 2109097): sec_len == -1 in pull_ucs2_base_talloc
> [2013/02/23 19:22:58.563334, 0] lib/util.c:921(log_stack_trace)
> BACKTRACE: 22 stack frames:
> #0 smbd(log_stack_trace+0x1a) [0x7f3dfe84022a]
> #1 smbd(smb_panic_s3+0x25) [0x7f3dfe8402f5]
> #2 smbd(smb_panic+0x1a1) [0x7f3dfe8321a1]
> #3 smbd(+0x466298) [0x7f3dfe82f298]
> #4 smbd(srvstr_get_path_wcard+0x42) [0x7f3dfe51a802]
> #5 smbd(srvstr_get_path_req_wcard+0x3c) [0x7f3dfe51a8cc]
> #6 smbd(srvstr_get_path_req+0x12) [0x7f3dfe51ba22]
> #7 smbd(reply_mkdir+0x53) [0x7f3dfe51bdf3]
> #8 smbd(+0x199a7b) [0x7f3dfe562a7b]
> #9 smbd(+0x19a864) [0x7f3dfe563864]
> #10 smbd(+0x19b709) [0x7f3dfe564709]
> #11 smbd(run_events_poll+0x376) [0x7f3dfe84f0d6]
> #12 smbd(+0x486580) [0x7f3dfe84f580]
> #13 smbd(_tevent_loop_once+0x90) [0x7f3dfe84f900]
> #14 smbd(smbd_process+0xc77) [0x7f3dfe561c07]
> #15 smbd(+0x7202dc) [0x7f3dfeae92dc]
> #16 smbd(run_events_poll+0x376) [0x7f3dfe84f0d6]
> #17 smbd(+0x486580) [0x7f3dfe84f580]
> #18 smbd(_tevent_loop_once+0x90) [0x7f3dfe84f900]
> #19 smbd(main+0x1381) [0x7f3dfeaead11]
> #20 /lib64/libc.so.6(__libc_start_main+0xfd) [0x7f3dfb396cdd]
> #21 smbd(+0x106a19) [0x7f3dfe4cfa19]
> [2013/02/23 19:22:58.563848, 0] lib/dumpcore.c:317(dump_core)
> dumping core in /var/log/samba/cores/smbd
>
> smb.base submits requests without any parameters and for a couple of SMB1 requests (SMBmkdir, SMBrmdir, SMBgetatr, SMBcheckpath, SMBfcloserequests) we try to access data behind req->buf+req->buflen, resulting in above panic.
>
> Attached patch set adds additional checks to prevent this invalid access.
Attached with my Review. Looks good to me, thanks! Someone else?
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 2b2ea490b7b76b95c88de2dd1279618c502662c0 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Thu, 4 Apr 2013 12:59:36 +0200
Subject: [PATCH 1/3] s3:smbd: do not access data behind req->buf+req->buflen in srvstr_get_path_req_wcard()
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/reply.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 0d9f415..5fb10d5 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -318,9 +318,16 @@ size_t srvstr_get_path_req_wcard(TALLOC_CTX *mem_ctx, struct smb_request *req,
char **pp_dest, const char *src, int flags,
NTSTATUS *err, bool *contains_wcard)
{
- return srvstr_get_path_wcard(mem_ctx, (const char *)req->inbuf, req->flags2,
- pp_dest, src, smbreq_bufrem(req, src),
- flags, err, contains_wcard);
+ ssize_t bufrem = smbreq_bufrem(req, src);
+
+ if (bufrem < 0) {
+ *err = NT_STATUS_INVALID_PARAMETER;
+ return 0;
+ }
+
+ return srvstr_get_path_wcard(mem_ctx, (const char *)req->inbuf,
+ req->flags2, pp_dest, src, bufrem, flags,
+ err, contains_wcard);
}
size_t srvstr_get_path_req(TALLOC_CTX *mem_ctx, struct smb_request *req,
--
1.7.0.4
From 18a651503e94b4703b54ca6e6d8e70a03a40468a Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Thu, 4 Apr 2013 13:24:36 +0200
Subject: [PATCH 2/3] s3:smbd: convert srvstr_pull_req_talloc() into a function
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/include/srvstr.h | 9 ---------
source3/smbd/proto.h | 2 ++
source3/smbd/reply.c | 11 +++++++++++
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/source3/include/srvstr.h b/source3/include/srvstr.h
index 7e7d8a2..2c6e7ef 100644
--- a/source3/include/srvstr.h
+++ b/source3/include/srvstr.h
@@ -19,12 +19,3 @@
#define srvstr_pull_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
pull_string_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
-
-/* pull a string from the smb_buf part of a packet. In this case the
- string can either be null terminated or it can be terminated by the
- end of the smbbuf area
-*/
-
-#define srvstr_pull_req_talloc(ctx, req_, dest, src, flags) \
- pull_string_talloc(ctx, req_->inbuf, req_->flags2, dest, src, \
- smbreq_bufrem(req_, src), flags)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 7e13049..2be73a2 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -841,6 +841,8 @@ size_t srvstr_get_path_req_wcard(TALLOC_CTX *mem_ctx, struct smb_request *req,
size_t srvstr_get_path_req(TALLOC_CTX *mem_ctx, struct smb_request *req,
char **pp_dest, const char *src, int flags,
NTSTATUS *err);
+size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req,
+ char **dest, const char *src, int flags);
bool check_fsp_open(connection_struct *conn, struct smb_request *req,
files_struct *fsp);
bool check_fsp(connection_struct *conn, struct smb_request *req,
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 5fb10d5..8c7fc16 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -339,6 +339,17 @@ size_t srvstr_get_path_req(TALLOC_CTX *mem_ctx, struct smb_request *req,
flags, err, &ignore);
}
+/* pull a string from the smb_buf part of a packet. In this case the
+ string can either be null terminated or it can be terminated by the
+ end of the smbbuf area
+*/
+size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req,
+ char **dest, const char *src, int flags)
+{
+ return pull_string_talloc(ctx, req->inbuf, req->flags2, dest, src,
+ smbreq_bufrem(req, src), flags);
+}
+
/****************************************************************************
Check if we have a correct fsp pointing to a file. Basic check for open fsp.
****************************************************************************/
--
1.7.0.4
From 7cd1acfa3234b464667bb27ac92bf6ad06110bd4 Mon Sep 17 00:00:00 2001
From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
Date: Thu, 4 Apr 2013 13:29:01 +0200
Subject: [PATCH 3/3] s3:smbd: do not access data behind req->buf+req->buflen in srvstr_pull_req_talloc()
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/reply.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 8c7fc16..2f3df3f 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -346,8 +346,14 @@ size_t srvstr_get_path_req(TALLOC_CTX *mem_ctx, struct smb_request *req,
size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req,
char **dest, const char *src, int flags)
{
+ ssize_t bufrem = smbreq_bufrem(req, src);
+
+ if (bufrem < 0) {
+ return 0;
+ }
+
return pull_string_talloc(ctx, req->inbuf, req->flags2, dest, src,
- smbreq_bufrem(req, src), flags);
+ bufrem, flags);
}
/****************************************************************************
--
1.7.0.4
More information about the samba-technical
mailing list