search

henrybear327 / Proton-API-Bridge

A third-party, open-source Proton API bridge (mainly focusing on Drive for now)
MIT License
82 stars 5 forks source link

SHA1 Case issue #21

Open henrybear327 opened 1 year ago

henrybear327 commented 1 year ago

https://github.com/rclone/rclone/issues/7345

tomekit commented 6 months ago

I was wondering if solution mentioned here: https://github.com/rclone/rclone/issues/7345#issuecomment-1821463100

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 commented 6 months ago

I've created pull request: https://github.com/henrybear327/Proton-API-Bridge/pull/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 commented 4 months ago

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 commented 2 months ago

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 commented 1 month ago

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 commented 1 month ago

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,