Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SHA1 Case issue #21

Open
henrybear327 opened this issue Sep 28, 2023 · 6 comments
Open

SHA1 Case issue #21

henrybear327 opened this issue Sep 28, 2023 · 6 comments
Assignees

Comments

@henrybear327
Copy link
Owner

rclone/rclone#7345

@henrybear327 henrybear327 self-assigned this Sep 28, 2023
@tomekit
Copy link
Contributor

tomekit commented Apr 29, 2024

I was wondering if solution mentioned here: rclone/rclone#7345 (comment)

Replace:
https://github.com/rclone/rclone/blob/master/fs/operations/operations.go#L119

return srcHash == dstHash, ht, srcHash, dstHash, nil

with

return strings.ToUpper(srcHash) == strings.ToUpper(dstHash), ht, srcHash, dstHash, nil

solves this problem?

tomekit added a commit to tomekit/Proton-API-Bridge that referenced this issue May 5, 2024
@tomekit
Copy link
Contributor

tomekit commented May 5, 2024

I've created pull request: #27 but given that I couldn't exactly reproduce this issue (no idea in which case Proton would return uppercase SHA1 instead of lowercased one) it is somewhat blind fix.

@henrybear327
Copy link
Owner Author

I will review it along with other changes that I have queued up on my side! Thank you for the PR!

Indeed, as you said, the way to test it is ... hard. What I usually do is upload files from rclone side and see if I can open it with web and iOS, and the other way around ...

@Acters
Copy link

Acters commented Sep 18, 2024

Instead of using == to string compare, it would be more semantically correct to use the strings.EqualFold() which assumes UTF-8 and will not convert full-casing. This is the perfect use for this!

For future reference, for complex Unicode cases, there is "third party" package from the golang devs: Fold

This is overkill, If you want to be extra safe with Unicode then the precis from the secure package subdir would be best.

@henrybear327
Copy link
Owner Author

Instead of using == to string compare, it would be more semantically correct to use the strings.EqualFold() which assumes UTF-8 and will not convert full-casing. This is the perfect use for this!

For future reference, for complex Unicode cases, there is "third party" package from the golang devs: Fold

This is overkill, If you want to be extra safe with Unicode then the precis from the secure package subdir would be best.

SHA-1 hash shouldn't have weird unicode issues, so I think using strings.EqualFold() should be sufficient!

Thank you for the references and ideas! :)

@Acters
Copy link

Acters commented Oct 10, 2024

Instead of using == to string compare, it would be more semantically correct to use the strings.EqualFold() which assumes UTF-8 and will not convert full-casing. This is the perfect use for this!
For future reference, for complex Unicode cases, there is "third party" package from the golang devs: Fold
This is overkill, If you want to be extra safe with Unicode then the precis from the secure package subdir would be best.

SHA-1 hash shouldn't have weird unicode issues, so I think using strings.EqualFold() should be sufficient!

Thank you for the references and ideas! :)

That is true, the only reason I mention precis is because the secure package is security focused. I believe that using methods that help enforce the input to be what we as programmers expect will help for a world where there is more security incidents occurring. probably paranoia, but I seldom see people care for writing code that doesn't have input sanitation or even using methods that help with enforcing types.(and nowadays it is just LLM generated that is just bad) Rust at least now is helping enforce memory safety and even has the type system enforcing code to be more strict with passing data around.

I think strings.EqualFold() is perfect and helps with not needing to implement our own folding. These are the kind packages/methods where many more eyes are looking into the implementation, allow spread of easily updated code for insecure implementations, and offer better code readability. I doubt something like this can be exploited but it is just my two cents to put any roadblocks to any unexpected execution/inputs,

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

No branches or pull requests

3 participants