vfs_default: fix vfswrap_offload_write_send() NT_STATUS_INVALID_VIEW_SIZE check
authorStefan Metzmacher <metze@samba.org>
Tue, 31 Jul 2018 10:29:29 +0000 (12:29 +0200)
committerKarolin Seeger <kseeger@samba.org>
Wed, 24 Apr 2019 07:00:30 +0000 (07:00 +0000)
This fixes a regression introduced in commit
60e45a2d25401eaf9a15a86d19114670ccfde259, where the 'num' variable
was renamed to 'to_copy', but a new 'num' variable was introduced.

Note that off_t is signed!
In future we need to watch out for filesystems supporting
FMODE_UNSIGNED_OFFSET on Linux. Which means they use it unsigned.

This is more or less a theoretical problem, The
NT_STATUS_INVALID_PARAMETER cases are catched before by
SMB_VFS_PREAD_SEND/RECV.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13862

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(cherry picked from commit 4d6cd932a955a99ca33cc4aedd7f612e56e0b1de)

source3/modules/vfs_default.c

index 7b57e9c71f966721c19ac6410f706c9caa6999a4..7dfa335190c8e824d1d6cf2e7cdec5f421d78d67 100644 (file)
@@ -1630,6 +1630,8 @@ static struct tevent_req *vfswrap_offload_write_send(
 {
        struct tevent_req *req;
        struct vfswrap_offload_write_state *state = NULL;
+       /* off_t is signed! */
+       off_t max_offset = INT64_MAX - to_copy;
        size_t num = MIN(to_copy, COPYCHUNK_MAX_TOTAL_LEN);
        files_struct *src_fsp = NULL;
        NTSTATUS status;
@@ -1681,6 +1683,35 @@ static struct tevent_req *vfswrap_offload_write_send(
                return tevent_req_post(req, ev);
        }
 
+       if (state->src_off > max_offset) {
+               /*
+                * Protect integer checks below.
+                */
+               tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+               return tevent_req_post(req, ev);
+       }
+       if (state->src_off < 0) {
+               /*
+                * Protect integer checks below.
+                */
+               tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+               return tevent_req_post(req, ev);
+       }
+       if (state->dst_off > max_offset) {
+               /*
+                * Protect integer checks below.
+                */
+               tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+               return tevent_req_post(req, ev);
+       }
+       if (state->dst_off < 0) {
+               /*
+                * Protect integer checks below.
+                */
+               tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+               return tevent_req_post(req, ev);
+       }
+
        status = vfs_offload_token_db_fetch_fsp(vfswrap_offload_ctx,
                                                token, &src_fsp);
        if (tevent_req_nterror(req, status)) {
@@ -1704,17 +1735,12 @@ static struct tevent_req *vfswrap_offload_write_send(
        state->src_ev = src_fsp->conn->user_ev_ctx;
        state->src_fsp = src_fsp;
 
-       state->buf = talloc_array(state, uint8_t, num);
-       if (tevent_req_nomem(state->buf, req)) {
-               return tevent_req_post(req, ev);
-       }
-
        status = vfs_stat_fsp(src_fsp);
        if (tevent_req_nterror(req, status)) {
                return tevent_req_post(req, ev);
        }
 
-       if (src_fsp->fsp_name->st.st_ex_size < state->src_off + num) {
+       if (src_fsp->fsp_name->st.st_ex_size < state->src_off + to_copy) {
                /*
                 * [MS-SMB2] 3.3.5.15.6 Handling a Server-Side Data Copy Request
                 *   If the SourceOffset or SourceOffset + Length extends beyond
@@ -1728,6 +1754,11 @@ static struct tevent_req *vfswrap_offload_write_send(
                return tevent_req_post(req, ev);
        }
 
+       state->buf = talloc_array(state, uint8_t, num);
+       if (tevent_req_nomem(state->buf, req)) {
+               return tevent_req_post(req, ev);
+       }
+
        status = vfswrap_offload_write_loop(req);
        if (!NT_STATUS_IS_OK(status)) {
                tevent_req_nterror(req, status);