Bug 15085 (CVE-2022-32742, ZDI-CAN-17388) - [SECURITY] CVE-2022-32742:SMB1 code does not correct verify SMB1write, SMB1write_and_close, SMB1write_and_unlock lengths.
Summary: [SECURITY] CVE-2022-32742:SMB1 code does not correct verify SMB1write, SMB1wr...
Status: RESOLVED FIXED
Alias: CVE-2022-32742, ZDI-CAN-17388
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.16.1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 15109
  Show dependency treegraph
 
Reported: 2022-06-07 00:13 UTC by Jeremy Allison
Modified: 2023-03-15 21:53 UTC (History)
9 users (show)

See Also:


Attachments
POC exploit (1.68 MB, application/octet-stream)
2022-06-07 00:13 UTC, Jeremy Allison
no flags Details
Raw patch for master. (614 bytes, patch)
2022-06-07 00:17 UTC, Jeremy Allison
no flags Details
git-am fix for master. (101 bytes, patch)
2022-06-07 16:55 UTC, Jeremy Allison
no flags Details
git-am fix for master. (5.09 KB, patch)
2022-06-07 16:56 UTC, Jeremy Allison
ddiss: review+
Details
Draft CVE announcement (2.06 KB, text/plain)
2022-06-07 19:41 UTC, Jeremy Allison
no flags Details
git-am fix for master. (5.12 KB, patch)
2022-06-07 22:19 UTC, Jeremy Allison
ddiss: review+
Details
Draft updated CVE text . (2.32 KB, text/plain)
2022-06-07 22:42 UTC, Jeremy Allison
ddiss: review+
Details
git-am fix for master. (5.91 KB, patch)
2022-06-08 19:32 UTC, Jeremy Allison
ddiss: review+
jra: ci-passed+
Details
git-am fix for 4.16.next, 4.15.next, 4.14.next (5.89 KB, patch)
2022-06-08 21:57 UTC, Jeremy Allison
ddiss: review+
Details
Proposed final CVE text. (2.32 KB, text/plain)
2022-06-09 16:38 UTC, Jeremy Allison
ddiss: review+
Details
git-am fix for master. (6.02 KB, patch)
2022-06-09 17:02 UTC, Jeremy Allison
ddiss: review+
abartlet: review-
Details
git-am fix for 4.16.next, 4.15.next, 4.14.next (6.00 KB, patch)
2022-06-09 17:14 UTC, Jeremy Allison
ddiss: review+
Details
git-am fix for 4.16.next. (5.99 KB, patch)
2022-06-14 16:44 UTC, Jeremy Allison
jra: review? (ddiss)
asn: review+
abartlet: review+
jra: ci-passed+
Details
git-am fix for 4.15.next. (5.99 KB, patch)
2022-06-14 17:48 UTC, Jeremy Allison
jra: review? (ddiss)
asn: review+
abartlet: review+
jra: ci-passed+
Details
updated avisory with CVSS 3.1 string and score only (2.24 KB, text/plain)
2022-06-21 00:19 UTC, Andrew Bartlett
jra: review+
Details
git-am fix for 4.14.next. (5.99 KB, patch)
2022-06-24 16:42 UTC, Jeremy Allison
jra: review? (ddiss)
asn: review+
abartlet: review+
jra: ci-passed+
Details
git-am fix for master. (6.02 KB, patch)
2022-07-19 00:31 UTC, Jeremy Allison
jra: review? (ddiss)
jra: ci-passed+
Details
git-am fix for 4.10 (6.00 KB, patch)
2022-07-20 08:13 UTC, Jo Sutton
abartlet: review+
jsutton: ci-passed+
Details
git-am fix for 4.13 (6.00 KB, patch)
2022-07-27 05:31 UTC, Jo Sutton
jsutton: ci-passed+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2022-06-07 00:13:08 UTC
Created attachment 17318 [details]
POC exploit

DI-CAN-17388: Samba SMB1 Out-Of-Bounds Read Information Disclosure Vulnera=
bility

-- CVSS -----------------------------------------

5.9: AV:N/AC:H/PR:L/UI:N/S:U/C:L/I:N/A:H

-- ABSTRACT -------------------------------------

Trend Micro's Zero Day Initiative has identified a vulnerability affecting =
the following products:
Samba - Samba

-- VULNERABILITY DETAILS ------------------------
* Version tested: 4.15.5-Ubuntu
* Installer file:
* Platform tested: ubuntu 22.04 desktop amd64

---

### Analysis

```

there are 2 pre-conditions:
1. A Read/write accessible shared folder(Change /etc/smb.conf to have the l=
ine "read only =3D no" under the [share name] section)
2. samba support SMB1(Change /etc/smb.conf to have "min protocol =3D NT1" u=
nder the [global] section.)

There is a heap oob read when samba server handles following SMB1 commands:

- SMB_COM_WRITE
- SMB_COM_WRITE_AND_CLOSE
- SMB_COM_WRITE_AND_UNLOCK

SMB_Parameters
{
    UCHAR  WordCount;
    Words
    {
        USHORT FID;
        USHORT CountOfBytesToWrite;
        ULONG  WriteOffsetInBytes;
        USHORT EstimateOfRemainingBytesToBeWritten;
    }
}
SMB_Data
{
    USHORT ByteCount;
    Bytes
    {
        UCHAR  BufferFormat;
        USHORT DataLength;
        UCHAR  Data[ CountOfBytesToWrite ];   =20
    }
}
```

Here is the annotated code from reply_write() (source3/smbd/smb1_reply.c) t=
hat handle SMB1's SMB_COM_WRITE. You can find the other same vulnerable cod=
e in reply_writeclose() and reply_writeunlock() in the same file.

```
void reply_write(struct smb_request *req) {
    connection_struct *conn =3D req->conn;
    size_t numtowrite;
    size_t remaining;

    /* ... */

    if (req->wct < 5) { // 1
    END_PROFILE(SMBwrite);
    reply_nterror(req, NT_STATUS_INVALID_PARAMETER);
    return;
    }

    /* ... */
    fsp =3D file_fsp(req, SVAL(req->vwv+0, 0)); // need FID here, so we nee=
d to open file first.

    /* ... */
    numtowrite =3D SVAL(req->vwv+1, 0); // number of bytes we want to write=
 from SMB_DATA->Data to remote file.
    startpos =3D IVAL_TO_SMB_OFF_T(req->vwv+2, 0);


    data =3D (const char *)req->buf + 3; // data points to SMB_DATA->Data

    remaining =3D smbreq_bufrem(req, data); //
    /*

    #define smbreq_bufrem(req, p) (req->buflen - PTR_DIFF(p, req->buf))

    PTR_DIFF(p,req->buf) =3D=3D PTR_DIFF(req->buf+3,req->buf) =3D=3D 3
    req->buflen =3D=3D SMB_DATA->ByteCount(value we can controlled in packe=
t)

    so, remaining =3D req->buflen - 3 // if req->buflen < 3, then it is an =
int underflow.

    */
    if (numtowrite > remaining) { // If there is an int underflow, then "re=
maining" will be a large value, so boundary check will fail.
        reply_nterror(req, NT_STATUS_INVALID_PARAMETER);
        END_PROFILE(SMBwrite);
        return;
    }

    /* ... */

    nwritten =3D write_file(req,fsp,data,startpos,numtowrite); // write num=
towrite(can be a large value because boundary check is failed) bytes from S=
MB_DATA->Data to remote file.
    /* ... */
}
```

It can be much more clear in ghidra.
```
buf =3D *(long *)(req + 0x38);
numtowrite =3D (ulong)*(ushort *)(*(long *)(req + 0x28) + 2);
startpos =3D *(undefined4 *)(*(long *)(req + 0x28) + 4);
/*
remaining =3D smbreq_bufrem(req, data);
if (numtowrite > remaining) {
    reply_nterror(req, NT_STATUS_INVALID_PARAMETER);
    END_PROFILE(SMBwrite);
    return;
}
*/
if ((ulong)*(ushort *)(req + 0x30) - 3 < numtowrite) { // if *(ulong*)(req+=
0x30) < 3, the value will become int underflow like(0xffffffff), req+0x30 =
=3D=3D req->buflen =3D=3D SMB_DATA->ByteCount
    ......
}

if (numtowrite =3D=3D 0) {
    ......
}
else {
    uVar7 =3D write_file(req,fsp,buf + 3,startpos,numtowrite); // numtowrit=
e can be large, so it is a heap OOB read
    ......
}
```
debug log
```
get the value from numtowrite, which is store in r14
get the value from req->buflen, which is store in rax
(gdb) x/10i $pc-0x10
   0x7f47bafbbfee <reply_write+398>:    mov    edx,DWORD PTR [rbp+0x38]
   0x7f47bafbbff1 <reply_write+401>:    movzx  r14d,WORD PTR [rax+0x2]
   0x7f47bafbbff6 <reply_write+406>:    mov    r15d,DWORD PTR [rax+0x4]
   0x7f47bafbbffa <reply_write+410>:    movzx  eax,WORD PTR [rbp+0x30]
=3D> 0x7f47bafbbffe <reply_write+414>:  mov    rbx,r14
   0x7f47bafbc001 <reply_write+417>:    sub    rax,0x3
   0x7f47bafbc005 <reply_write+421>:    cmp    r14,rax
   0x7f47bafbc008 <reply_write+424>:    ja     0x7f47bafbc170 <reply_write+784>
   0x7f47bafbc00e <reply_write+430>:    cmp    QWORD PTR [r12+0x178],0x0
   0x7f47bafbc017 <reply_write+439>:    je     0x7f47bafbc1e0 <reply_write+896>
(gdb) x/gx $rax
0x0:    Cannot access memory at address 0x0
(gdb) x/gx $r14
0xffff: Cannot access memory at address 0xffff

sub req->buflen with 3, result will be in rax.
(gdb) x/10i $pc
=3D> 0x7f47bafbc001 <reply_write+417>:  sub    rax,0x3
   0x7f47bafbc005 <reply_write+421>:    cmp    r14,rax
   0x7f47bafbc008 <reply_write+424>:    ja     0x7f47bafbc170 <reply_write+784>
   0x7f47bafbc00e <reply_write+430>:    cmp    QWORD PTR [r12+0x178],0x0
   0x7f47bafbc017 <reply_write+439>:    je     0x7f47bafbc1e0 <reply_write+896>
   0x7f47bafbc01d <reply_write+445>:    test   rbx,rbx
   0x7f47bafbc020 <reply_write+448>:    jne    0x7f47bafbbf20 <reply_write+192>
   0x7f47bafbc026 <reply_write+454>:    mov    rsi,r15
   0x7f47bafbc029 <reply_write+457>:    mov    rdi,r12
   0x7f47bafbc02c <reply_write+460>:    call   0x7f47baff2750 <vfs_allocate_fi=
le_space>
(gdb) ni
0x00007f47bafbc005 in reply_write () from /usr/lib/x86_64-linux-gnu/samba/l=
ibsmbd-base.so.0
(gdb) x/gx $rax // int underflow here.
0xfffffffffffffffd:     Cannot access memory at address 0xfffffffffffffffd
(gdb) x/10i $pc
=3D> 0x7f47bafbc005 <reply_write+421>:  cmp    r14,rax
   0x7f47bafbc008 <reply_write+424>:    ja     0x7f47bafbc170 <reply_write+784>
   0x7f47bafbc00e <reply_write+430>:    cmp    QWORD PTR [r12+0x178],0x0
   0x7f47bafbc017 <reply_write+439>:    je     0x7f47bafbc1e0 <reply_write+896>
   0x7f47bafbc01d <reply_write+445>:    test   rbx,rbx
   0x7f47bafbc020 <reply_write+448>:    jne    0x7f47bafbbf20 <reply_write+192>
   0x7f47bafbc026 <reply_write+454>:    mov    rsi,r15
   0x7f47bafbc029 <reply_write+457>:    mov    rdi,r12
   0x7f47bafbc02c <reply_write+460>:    call   0x7f47baff2750 <vfs_allocate_fi=
le_space>
   0x7f47bafbc031 <reply_write+465>:    test   eax,eax
(gdb) x/gx $r14
0xffff: Cannot access memory at address 0xffff
(gdb) x/gx $rax
0xfffffffffffffffd:     Cannot access memory at address 0xfffffffffffffffd

(gdb) x/10i $pc
=3D> 0x7f47bafbbf30 <reply_write+208>:  call   0x7f47baf99c60 <write_file>
   0x7f47bafbbf35 <reply_write+213>:    mov    rdi,r13
   0x7f47bafbbf38 <reply_write+216>:    xor    edx,edx
   0x7f47bafbbf3a <reply_write+218>:    mov    rsi,r12
   0x7f47bafbbf3d <reply_write+221>:    mov    r15,rax
   0x7f47bafbbf40 <reply_write+224>:    call   0x7f47baf99e70 <sync_file>
   0x7f47bafbbf45 <reply_write+229>:    mov    r13d,eax
   0x7f47bafbbf48 <reply_write+232>:    test   eax,eax
   0x7f47bafbbf4a <reply_write+234>:    jne    0x7f47bafbc13e <reply_write+734>
   0x7f47bafbbf50 <reply_write+240>:    test   r15,r15
(gdb) x/gx $rdx // address of SMB_DATA->Data
0x560d1caf3ae4: 0x0000000000001262
(gdb) x/gx $r8 // numtowrite
0xffff: Cannot access memory at address 0xffff
(gdb) x/100b $rdx
0x560d1caf3ae4: 0x62    0x12    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3aec: 0x00    0x00    0x00    0x00    0x98    0x1a    0x77    0x94
0x560d1caf3af4: 0x00    0x00    0x5c    0x00    0x00    0x00    0x00    0x00
0x560d1caf3afc: 0x00    0x00    0x00    0x00    0xd0    0x00    0xaf    0x1c
0x560d1caf3b04: 0x0d    0x56    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b0c: 0x00    0x00    0x00    0x00    0x50    0x3a    0xaf    0x1c
0x560d1caf3b14: 0x0d    0x56    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b1c: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b24: 0x00    0x00    0x00    0x00    0xf1    0x8a    0x0e    0xbb
0x560d1caf3b2c: 0x47    0x7f    0x00    0x00    0xc8    0x00    0x00    0x00
0x560d1caf3b34: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b3c: 0x00    0x00    0x00    0x00    0xd0    0x39    0xaf    0x1c
0x560d1caf3b44: 0x0d    0x56    0x00    0x00
(gdb)=20
0x560d1caf3b48: 0xd8    0xea    0x10    0xbb    0x47    0x7f    0x00    0x00
0x560d1caf3b50: 0x0b    0x00    0x41    0xc8    0xe1    0x67    0x00    0x00
0x560d1caf3b58: 0x06    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b60: 0x00    0x00    0x00    0x00    0x0d    0x56    0x00    0x00
0x560d1caf3b68: 0x51    0x42    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b70: 0x55    0x19    0x00    0x00    0x05    0x67    0x00    0x00
0x560d1caf3b78: 0xd5    0x3a    0xaf    0x1c    0x0d    0x56    0x00    0x00
0x560d1caf3b80: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3b88: 0xe1    0x3a    0xaf    0x1c    0x0d    0x56    0x00    0x00
0x560d1caf3b90: 0xb0    0x3a    0xaf    0x1c    0x0d    0x56    0x00    0x00
0x560d1caf3b98: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3ba0: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0x560d1caf3ba8: 0x00    0x3a    0xaf    0x1c


The result of the exploit.
$ xxd -g 1 leaks | head -n 30
00000000: 62 12 00 00 00 00 00 00 00 00 00 00 98 1a 77 94  b.............w.
00000010: 00 00 5c 00 00 00 00 00 00 00 00 00 d0 00 af 1c  ..\.............
00000020: 0d 56 00 00 00 00 00 00 00 00 00 00 50 3a af 1c  .V..........P:..
00000030: 0d 56 00 00 00 00 00 00 00 00 00 00 00 00 00 00  .V..............
00000040: 00 00 00 00 f1 8a 0e bb 47 7f 00 00 c8 00 00 00  ........G.......
00000050: 00 00 00 00 00 00 00 00 00 00 00 00 d0 39 af 1c  .............9..
00000060: 0d 56 00 00 d8 ea 10 bb 47 7f 00 00 0b 00 41 c8  .V......G.....A.
00000070: e1 67 00 00 06 00 00 00 00 00 00 00 00 00 00 00  .g..............
00000080: 0d 56 00 00 51 42 00 00 00 00 00 00 55 19 00 00  .V..QB......U...
00000090: 05 67 00 00 d5 3a af 1c 0d 56 00 00 00 00 00 00  .g...:...V......
000000a0: 00 00 00 00 e1 3a af 1c 0d 56 00 00 b0 3a af 1c  .....:...V...:..
000000b0: 0d 56 00 00 00 00 00 00 00 00 00 00 00 00 00 00  .V..............
000000c0: 00 00 00 00 00 3a af 1c 0d 56 00 00 a0 ae aa 1c  .....:...V......
000000d0: 0d 56 00 00 30 2c ad 1c 0d 56 00 00 50 9a ae 1c  .V..0,...V..P...
000000e0: 0d 56 00 00 40 b4 ae 1c 0d 56 00 00 00 00 00 00  .V..@....V......
000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 b0 03 af 1c  ................
00000100: 0d 56 00 00 40 b4 ae 1c 0d 56 00 00 00 00 00 00  .V..@....V......
00000110: 00 00 00 00 00 00 00 00 00 00 00 00 4e 17 72 62  ............N.rb
00000120: 00 00 00 00 0d f5 05 00 00 00 00 00 00 00 00 00  ................
00000130: 00 00 00 00 00 00 00 00 00 00 00 00 7b ef 17 ea  ............{...
00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000170: 00 00 00 00 17 14 20 ba 47 7f 00 00 98 00 00 00  ...... .G.......
00000180: 00 00 00 00 00 00 00 00 00 00 00 00 d0 39 af 1c  .............9..
00000190: 0d 56 00 00 78 c3 ac ba 47 7f 00 00 00 00 00 00  .V..x...G.......
000001a0: 00 00 00 00 a0 b4 52 14 fd 7f 00 00 20 00 00 00  ......R..... ...
000001b0: 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   ...............
000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
000001d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................


```


-- CREDIT ---------------------------------------
This vulnerability was discovered by:
Luca Moro (@johncool__) working with Trend Micro Zero Day Initiative

-- FURTHER DETAILS ------------------------------

If supporting files were contained with this report they are provided withi=
n a password protected ZIP file. The password is the ZDI candidate number i=
n the form: ZDI-CAN-XXXX where XXXX is the ID number.

Please confirm receipt of this report. We expect all vendors to remediate Z=
DI vulnerabilities within 120 days of the reported date. If you are ready t=
o release a patch at any point leading up to the deadline, please coordinat=
e with us so that we may release our advisory detailing the issue. If the 1=
20-day deadline is reached and no patch has been made available we will rel=
ease a limited public advisory with our own mitigations, so that the public=
 can protect themselves in the absence of a patch. Please keep us updated r=
egarding the status of this issue and feel free to contact us at any time:

Zero Day Initiative
zdi-disclosures@trendmicro.com
Comment 1 Jeremy Allison 2022-06-07 00:17:48 UTC
Created attachment 17319 [details]
Raw patch for master.

So I don't lose it. Fixes the supplied exploit.
Comment 2 Jeremy Allison 2022-06-07 16:13:56 UTC
I have a test for this that reproduces the supplied POC in smbtorture. Will get the patches finished hopefully today.
Comment 3 Jeremy Allison 2022-06-07 16:55:32 UTC
Created attachment 17322 [details]
git-am fix for master.

Fix including regression test that reproduces the supplied proof-of-concept.
Comment 4 Jeremy Allison 2022-06-07 16:56:37 UTC
Created attachment 17323 [details]
git-am fix for master.

Now with the actual code :-).
Comment 5 Jeremy Allison 2022-06-07 19:41:03 UTC
Created attachment 17324 [details]
Draft CVE announcement

Needs CVE number adding.
Comment 6 David Disseldorp 2022-06-07 21:50:54 UTC
Comment on attachment 17323 [details]
git-am fix for master.

Looks fine.
Reviewed-by: David Disseldorp <ddiss@samba.org>

716         fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE);
717         if (fnum == -1) {
718                 torture_fail_goto(tctx,
719                         done,
...
773 done:
774         if (req != NULL) {
775                 smbcli_request_destroy(req);
776         }
777         smbcli_close(cli->tree, fnum);

It's only test code, so can definitely stay as-is, but fwiw this will send an SMBclose(fnum=-1) on open failure...
Comment 7 Jeremy Allison 2022-06-07 22:03:17 UTC
(In reply to David Disseldorp from comment #6)

Pedant :-). It's early enough I'll fix the test code :-).
Comment 8 Jeremy Allison 2022-06-07 22:19:33 UTC
Created attachment 17325 [details]
git-am fix for master.

Fixed up the fnum==-1 means no close issue as David pointed out.

Thanks for noticing in the test code :-).
Comment 9 David Disseldorp 2022-06-07 22:26:31 UTC
Comment on attachment 17324 [details]
Draft CVE announcement

Looks fine, a few minor nits:

> Please note that SMB1 is *NOT* enabled by default, only sites where
> SMB1 has been enabled by the administrator are vulnerable to this bug.

I think it's worth also mentioning the Samba version >= 4.11.0 caveat
here too.

> This is an SMB1-only vulnerability. Since Samba release 4.11.0 SMB1
> has been disabled by default. Do not turn on SMB1 serving unless
> required for your specific clients.

"unless required for your specific clients" can probably be dropped. How
about something like "We do not recommend enabling SMB1 server support"?
Comment 10 Jeremy Allison 2022-06-07 22:42:52 UTC
Created attachment 17326 [details]
Draft updated CVE text .

Updated with suggestions from David.
Comment 11 David Disseldorp 2022-06-08 07:53:11 UTC
Comment on attachment 17326 [details]
Draft updated CVE text .

Looks good. Now just waiting on proper CVE ID#.
Comment 12 Jeremy Allison 2022-06-08 19:32:48 UTC
Created attachment 17327 [details]
git-am fix for master.

Sorry for the re-review David, but the original patch fails on the -O3 build due to the following reason:

Once smbreq_bufrem(req, p) is modified to return 0 when p is beyond the buffer length, the tests inside source3/smbd/smb2_reply.c:srvstr_get_path_req() and source3/smbd/smb2_reply.c:srvstr_pull_req_talloc() are not useful anymore, as the compiler knows (smbreq_bufrem(req, p) < 0) is always false and so complains about the check.

Simply change the tests to be:

if (smbreq_bufrem(req, p) == 0)

instead, as at that point we know we're at the end or beyond of the supplied buffer, and the client is misbehaving.
Comment 13 David Disseldorp 2022-06-08 19:54:51 UTC
Comment on attachment 17327 [details]
git-am fix for master.

Oops, I should have caught that in the first review. The updated fix looks good, thanks.
Comment 14 Jeremy Allison 2022-06-08 20:01:12 UTC
(In reply to David Disseldorp from comment #13)

No worries David, I only caught this in a private autobuild. It's a subtle change, so thanks for the timely review :-).
Comment 15 Jeremy Allison 2022-06-08 20:36:33 UTC
Comment on attachment 17327 [details]
git-am fix for master.

Passed private security autobuild.
Comment 16 Jeremy Allison 2022-06-08 21:57:31 UTC
Created attachment 17329 [details]
git-am fix for 4.16.next, 4.15.next, 4.14.next

Back-ported from master.
Comment 17 Jeremy Allison 2022-06-09 16:38:13 UTC
Created attachment 17334 [details]
Proposed final CVE text.

Added correct CVE number and proposed 4.16.next, 4.15.next, 4.14.next numbers. If these are correct this text is good to go.
Comment 18 Jeremy Allison 2022-06-09 17:02:56 UTC
Created attachment 17335 [details]
git-am fix for master.

Added RB+ and CVE number. No code changes. This should be good for master.
Comment 19 Jeremy Allison 2022-06-09 17:14:21 UTC
Created attachment 17336 [details]
git-am fix for 4.16.next, 4.15.next, 4.14.next

Updated with RB+ tag and CVE number. No code changes. I think this is good to go for the release.
Comment 20 Andrew Bartlett 2022-06-14 02:49:48 UTC
Comment on attachment 17336 [details]
git-am fix for 4.16.next, 4.15.next, 4.14.next

CVE tag should be in the first line please, eg

CVE-2022-32742 s3: smbd: Harden the smbreq_bufrem() macro.

Also one file per version, even if the same.  This allows you to tick a separate ci-passed on each version once they pass autouild.

https://wiki.samba.org/index.php/Samba_Security_Process has the steps.
Comment 21 Jeremy Allison 2022-06-14 16:44:38 UTC
Created attachment 17348 [details]
git-am fix for 4.16.next.

Same patch already reviewed by ddiss@samba.org. Updated text with CVE reference moved as Andrew requested. Passes private autobuild.
Comment 22 Jeremy Allison 2022-06-14 17:48:43 UTC
Created attachment 17349 [details]
git-am fix for 4.15.next.

Same patch already reviewed by ddiss@samba.org. Updated text with CVE reference moved as Andrew requested. Passes private autobuild.
Comment 23 Jeremy Allison 2022-06-14 19:05:29 UTC
I can't get 4.14.next to build due to the autobuild host compiler needing the -Wno-format-truncation flags to avoid truncation warnings.

Let me know if we need a 4.14.next build, and I'll work with Jule to get the pre-requisite patches back ported.
Comment 24 Andrew Bartlett 2022-06-21 00:05:14 UTC
(In reply to Jeremy Allison from comment #23)
We will need this building and fully tested in autobuild on Samba 4.14, otherwise the combined autobuild will of course fail. 

So any other patches this needs must be backported, the easiest way to schedule that is to include them in the backport here. 

But I'm confused, I don't see anything obvious that would be doing interesting things around format truncation, can you explain more?

Thanks,
Comment 25 Andrew Bartlett 2022-06-21 00:19:57 UTC
Created attachment 17378 [details]
updated avisory with CVSS 3.1 string and score only

I was confused by the 2.9 score, which seemed wrong so I re-scored this with https://www.first.org/cvss/calculator/3.1#CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N and got 4.3 which makes more sense to me for the issue.

I don't see why the attack complexity would be low (not hard to get some memory at all), but think that the integrity score should be low given the lack of control on what memory is written.
Comment 26 Jeremy Allison 2022-06-21 02:40:57 UTC
(In reply to Andrew Bartlett from comment #24)

When I schedule a private autobuild on the 'autobuild' host after checking out v4-14 and applying these patches the build fails:

$ cd my-samba-wip/
$ git checkout samba-4.4.14
$ git am ~/4.14.patches
$ autobuild-private-security.sh >&~/output

Fails with:

==> samba.stderr <==
../lib/replace/test/testsuite.c: In function ‘test_snprintf’:
../lib/replace/test/testsuite.c:355:6: error: ‘%d’ directive output truncated writing 1 byte into a region of size 0 [-Werror=format-truncation=]
  if (snprintf(tmp, 3, "foo%d", 9) != 4) {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/replace/test/testsuite.c:355:6: note: ‘snprintf’ output 5 bytes into a destination of size 3

I don't understand why this has changed since the last working 4.14 build.

Could this be due to a compiler change ?
Comment 27 Jeremy Allison 2022-06-21 02:43:57 UTC
Am I running the private autobuild incorrectly for 4.14 ?

Can someone explain how I should be running this ?

The code it's complaining about isn't being changed in my patches, and so the build should succeed as did the last 4.14 build.
Comment 28 Jeremy Allison 2022-06-24 16:42:00 UTC
Created attachment 17393 [details]
git-am fix for 4.14.next.

Passes ci on catalyst security.
Comment 29 Andrew Bartlett 2022-06-28 07:31:16 UTC
It would be good to get a review+ on the rest of the patches here, to have this all set up for the next security release.
Comment 30 Jeremy Allison 2022-06-30 21:07:44 UTC
I'm going to be out on vacation for the next 2 weeks, so adding Andreas in the hope he can give +1 to the (simple) patches for this.
Comment 31 Andreas Schneider 2022-07-01 07:09:57 UTC
ssize_t bufrem = smbreq_bufrem(req, src);

if (bufrem == 0)
...

It looks like bufrem can't be negative anymore, should the type be size_t now?
Comment 32 Jeremy Allison 2022-07-01 15:44:39 UTC
> It looks like bufrem can't be negative anymore, should the type be size_t now?

This is too big a change for a security fix, as it would require changes to called functions pull_string_talloc() and srvstr_get_path_internal() also, which are called in 35 other places.

Leaving as ssize_t (and checking for zero only) is the minimal fix (and David was also OK with it :-).
Comment 33 David Disseldorp 2022-07-01 18:44:36 UTC
(In reply to Jeremy Allison from comment #32)
> > It looks like bufrem can't be negative anymore, should the type be size_t now?
> 
> This is too big a change for a security fix, as it would require changes to
> called functions pull_string_talloc() and srvstr_get_path_internal() also,
> which are called in 35 other places.
> 
> Leaving as ssize_t (and checking for zero only) is the minimal fix (and
> David was also OK with it :-).

Yep. I'll take look at the backports tonight, sorry about the wait.
Comment 34 Andrew Bartlett 2022-07-14 04:15:18 UTC
Comment on attachment 17335 [details]
git-am fix for master.

Patch for master needs to be fixed to have CVE tag in commit subject.  Otherwise fine.
Comment 35 Andrew Bartlett 2022-07-14 04:18:11 UTC
Opening security bugs to vendors.  Release date is currently proposed to be Wednesday 27 July but bug 15109 will be the authoritative reference on that.
Comment 36 Jeremy Allison 2022-07-19 00:31:29 UTC
Created attachment 17427 [details]
git-am fix for master.

Updated master patch to put CVE number in Subject: line. No other changes.
Comment 37 Jo Sutton 2022-07-20 08:13:30 UTC
Created attachment 17434 [details]
git-am fix for 4.10
Comment 38 Jo Sutton 2022-07-27 05:31:01 UTC
Created attachment 17447 [details]
git-am fix for 4.13
Comment 39 Samba QA Contact 2022-07-27 10:32:42 UTC
This bug was referenced in samba v4-16-stable (Release samba-4.16.4):

ed3f82f4d70bbc89b89af31153eed96a544a754a
74946420dd59a102c8d5f4a0127d5e479da5470d
Comment 40 Samba QA Contact 2022-07-27 10:34:56 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.14):

f6e1750c4fc966c29c2e0663d3c04e87057fa0c3
7720e0acfd7ea6a2339f3e389aa8dcedd6174095
Comment 41 Samba QA Contact 2022-07-27 10:35:14 UTC
This bug was referenced in samba v4-15-stable (Release samba-4.15.9):

d6aef6838a674ab95ff9172f4ac67707667f9e00
a4707e4a955d01edf493cd0d7ab8b1ecb4ca7991
Comment 42 Samba QA Contact 2022-07-27 10:39:01 UTC
This bug was referenced in samba v4-14-test:

f6e1750c4fc966c29c2e0663d3c04e87057fa0c3
7720e0acfd7ea6a2339f3e389aa8dcedd6174095
Comment 43 Jule Anger 2022-07-27 11:07:01 UTC
Removing vendor CC (so that any public comments don't need to be broadcast so widely) and opening these bugs to the public.
If you wish to continue to be informed about any changes here please CC individually.
Comment 44 Samba QA Contact 2022-07-27 11:11:13 UTC
This bug was referenced in samba v4-16-test:

ed3f82f4d70bbc89b89af31153eed96a544a754a
74946420dd59a102c8d5f4a0127d5e479da5470d
Comment 45 Samba QA Contact 2022-07-27 11:11:23 UTC
This bug was referenced in samba v4-15-test:

d6aef6838a674ab95ff9172f4ac67707667f9e00
a4707e4a955d01edf493cd0d7ab8b1ecb4ca7991
Comment 46 Samba QA Contact 2022-07-27 11:59:01 UTC
This bug was referenced in samba master:

a60863458dc6b60a09aa8d31fada6c36f5043c76
3ddc9344c2fa7461336899fbddb0bb80995e9170
Comment 47 Samba QA Contact 2022-07-27 13:06:55 UTC
This bug was referenced in samba v4-17-stable:

a60863458dc6b60a09aa8d31fada6c36f5043c76
3ddc9344c2fa7461336899fbddb0bb80995e9170
Comment 48 Samba QA Contact 2022-07-27 13:07:45 UTC
This bug was referenced in samba v4-17-test:

a60863458dc6b60a09aa8d31fada6c36f5043c76
3ddc9344c2fa7461336899fbddb0bb80995e9170