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)
committerJeremy Allison <jra@samba.org>
Thu, 28 Mar 2019 23:09:35 +0000 (23:09 +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>
source3/modules/vfs_default.c

index 0ac67d5783536e9522f14d03f6090a8f28baea14..47722d53cec2634574d60a856d8397b9c61559c2 100644 (file)
@@ -1802,6 +1802,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;
@@ -1853,6 +1855,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)) {
@@ -1876,17 +1907,12 @@ static struct tevent_req *vfswrap_offload_write_send(
        state->src_ev = src_fsp->conn->sconn->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
@@ -1900,6 +1926,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);