Skip to content

fix(smb): migrate to cloudsoda go-smb2 - #9617

Open
mengskysama wants to merge 1 commit into
AlistGo:mainfrom
mengskysama:fix/smb
Open

fix(smb): migrate to cloudsoda go-smb2#9617
mengskysama wants to merge 1 commit into
AlistGo:mainfrom
mengskysama:fix/smb

Conversation

@mengskysama

Copy link
Copy Markdown

What

Migrates the SMB driver from the unmaintained github.com/hirochachacha/go-smb2 module to the actively maintained github.com/cloudsoda/go-smb2 fork used by rclone.

The old library used server-provided fields such as FileNameLength and NextEntryOffset for slicing SMB directory response buffers without sufficient bounds validation.

This brings in upstream SMB protocol hardening and prevents malformed directory responses from crashing AList with errors sometime such as:

状态
getting src object
错误
runtime error: slice bounds out of range [:10569] with capacity 144

Changes

  • Replaces github.com/hirochachacha/go-smb2 with a pinned github.com/cloudsoda/go-smb2 revision.
  • Adapts the SMB connection code to the new context-aware dial API.

Notes

  • Existing SMB storage configuration and authentication behavior remain unchanged.
  • The dependency is pinned to a specific commit instead of tracking a moving branch.

@okatu-loli

Copy link
Copy Markdown
Collaborator

The migration itself is clean, and threading ctx through checkConn/initFS reads right. cloudsoda is the fork rclone uses, so this is a good direction, and it should take care of the slice-bounds crash people were hitting.

Two things before this goes in:

  1. I want to see CI green. I cant verify the new Dial(ctx, addr) signature locally without vendoring the module, so Im leaning on the build for that.
  2. Heads up that this pulls in the whole jcmturner kerberos tree plus gorilla/sessions and gorilla/securecookie as transitive deps. Thats expected baggage from the cloudsoda fork, just flagging that the dependency surface grows a fair bit.

Assuming CI passes, this is good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants