Created attachment 18825 [details]
Patch for master

Created attachment 18826 [details]
Patch for 4.24

Created attachment 18827 [details]
Patch for 4.23

Created attachment 18828 [details]
Patch for 4.22

Created attachment 18829 [details]
CVE advisory

Comment on attachment 18829 [details]
CVE advisory

The test looks good.

I'd give it

Vector: AV:N/AC:L/PR:H/UI:N/S:U/C:N/I:H/A:N
Score: 4.9 (Medium)

instead of

Vector: AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:N
Score: 4.3 (Medium)

The difference are:
 
Privileges Required (PR) is high,
as the user needs to have write permissions to
the unix filesystem, which a typical read-only share
doesn't have.

Integrity Impact (I) is high,
as the attacker may redirect the user to a different
file controlled by the attacker.
And the attacker is able to block access to the file
content.

Comment on attachment 18825 [details]
Patch for master

Both MS-FSA 2.1.5.10.3 FSCTL_DELETE_REPARSE_POINT and
2.1.5.10.37 FSCTL_SET_REPARSE_POINT have this:

- If (Open.GrantedAccess & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES)) == 0, the
  operation MUST be failed with STATUS_ACCESS_DENIED.
- If Open.File.Volume.IsReadOnly is TRUE, the operation MUST be failed with
  STATUS_MEDIA_WRITE_PROTECTED.

I don't this we need STATUS_MEDIA_WRITE_PROTECTED,
as rejected_share_access = access_mask & ~(fsp->conn->share_access)
in smbd_calculate_access_mask_fsp() should already deny any open
with FILE_WRITE_DATA or FILE_WRITE_ATTRIBUTES, see create_share_access_mask()

But I guess in order to pass the existing tests
the check needs to allow FILE_WRITE_ATTRIBUTES.

I guess this heck would do what we need:

if (fsp->fsp_name->twrp != 0 ||
    !(fsp->access_mask & (FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES)))
{

Maybe the twrp check may not be needed, but it feels better this
way until we have explicit tests for the condition.

And 4.21 still gets security fixes as 4.24.0 is not released yet

Unfortunately the patches break the build, so more work is needed.

(In reply to Stefan Metzmacher from comment #6)

> Privileges Required (PR) is high,
> as the user needs to have write permissions to
> the unix filesystem, which a typical read-only share
> doesn't have.
> 
> Integrity Impact (I) is high,
> as the attacker may redirect the user to a different
> file controlled by the attacker.
> And the attacker is able to block access to the file
> content.

With your additional insight that an attacker can easily make an entire file system at least temporarily unavailable, I end up at

AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:H
Overall CVSS Score: 7.1

I leave PR at low, because my understanding is that PR:H means the attacker needs admin privileges whereas PR:L means normal user privs.

Created attachment 18830 [details]
Next version, it's a CVSS 7.1 (High)

Created attachment 18831 [details]
Patch for master

Created attachment 18833 [details]
Patch for 4.24

Created attachment 18835 [details]
Patch for 4.23

Comment on attachment 18830 [details]
Next version, it's a CVSS 7.1 (High)

Looks good thanks!

Comment on attachment 18831 [details]
Patch for master

The changes look good, thanks! They get a review+ when the 'WIP:' is removed.

(In reply to Stefan Metzmacher from comment #16)
Use check_any_access_fsp()? What about other ioctls? Any others maybe missing the access check?

(In reply to Ralph Böhme from comment #17)

I knew we had some helper function, but I couldn't find it.

I guess check_any_access_fsp() is exactly what we want :-)

(In reply to Stefan Metzmacher from comment #18)

source3/smbd/smb2_ioctl_filesys.c already has some check_any_access_fsp()
calls.

Maybe it would be good move the checks there in order
to avoid future problems if other vfs modules want to implement
reparse points. But that can be done as cleanup after a security
release.

(In reply to Stefan Metzmacher from comment #19)

Even the use of check_any_access_fsp() could be a future cleanup fix,
as a security fix the current fix seems to be enough.

Created attachment 18836 [details]
Patch for 4.22

Created attachment 18837 [details]
Patch for 4.21

Created attachment 18838 [details]
Changed advisory workaround wording

Created attachment 18839 [details]
Patch without WIP: and with Reviewed-by: tags

The attached patch is functionally the same, it applies to all affected branches. Only for 4.21 "git am -3" has to slightly massage the line numbers. If needed and as mandated by the security process, I can upload individual ones.

Handing over to the release account for further security release processing.

Adding original bug reporter for review

Hi folks, thanks for adding me here. I've re-validated the patch and confirmed it works as intended; the writeup looks good as well. I appreciate your callout of the higher severity -- I had not appreciated the full consequences of the issue. 

Good to proceed on my end! Thanks

Comment on attachment 18839 [details]
Patch without WIP: and with Reviewed-by: tags

Björn, please use this for all branches, the others are there
to show ci passed

scheduled release date for the security releases is 2026-03-05

hey all, just checking if 3/5 is still the target or if the date has changed. Thanks

release date was postponed because there are other security fixes that are supposed to get fixed with the same sec release.

Ack, thanks Björn! Do you have a new target date in mind already, or is it up in the air?

Created attachment 18883 [details]
Patch for v4.21 with reviewed-by, without WIP:

v4-21 patch is slightly different.

Hi folks, just wanted to check once more if you had a new target date in mind -- no rush *at all*, just wanted to include this finding into a writeup of mine at some point, so want to align on dates. Apologies if this is answered by the linked issues, unfortunately I can't see them.

we have more security releaste fixes in the pipe, which we queue into the next sec release. I expect in about 2 weeks we'll be able to make a release for all those issues.

the security release that was scheduled for tomorrow, will be postponed due to
new problems that have been identified with one of the fixes.

We will announce a new release date as soon as possible after the remaining
issues have been ruled out.

Hi Björn, thanks for the heads up. Do you have a rough date for the release in mind?

(In reply to Asim Viladi Oglu Manizada from comment #37)
as soon as we can oversee that the fixes for the vulnerabilities are in good shape, we set a date and the release date will be published then. There is no estimated release date we can tell you in advance until the patches are ready. The announcement will be made here and in all the related bug reports and on the mailing lists then.

Scheduled release date is now 2026-05-26.

I plan to upload the releases in about 3 hours from now...

This bug was referenced in samba v4-24-stable (Release samba-4.24.3):

ffbccc5d9f08469f42932db595c7eb886d9064bd
a53a86e1167d76cfc0c051a50b5f372afc78b721

This bug was referenced in samba v4-23-stable (Release samba-4.23.8):

39856d437502dfd7e80ac2d6a1690e45b636056e
7443fb5ad3f7efe4d9061320c29dbbfd636a41af

This bug was referenced in samba v4-22-stable (Release samba-4.22.10):

024c0f6e511f0b477f5dd2582f03ac0f44f011d1
a0e66dbfc549b7b3daea5b82a626dda5810ec764

This bug was referenced in samba v4-24-test (Release samba-4.24.3):

ffbccc5d9f08469f42932db595c7eb886d9064bd
a53a86e1167d76cfc0c051a50b5f372afc78b721

This bug was referenced in samba v4-23-test (Release samba-4.23.8):

39856d437502dfd7e80ac2d6a1690e45b636056e
7443fb5ad3f7efe4d9061320c29dbbfd636a41af

This bug was referenced in samba v4-22-test (Release samba-4.22.10):

024c0f6e511f0b477f5dd2582f03ac0f44f011d1
a0e66dbfc549b7b3daea5b82a626dda5810ec764

This bug was referenced in samba master:

fb8ef1c97e1ce36d633b46cf10ff48aa70c2ed4c
6c7f7c40029b9d34b1ede83391cc451a483cf1d2