fix(sec): randomString bias

[trim21]

security issue added by #2490

len("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz")==52, and 256 = 52 * 4 + 48, so the possibility of each characters generated by randomString is not equal.

A-Za-v: 5/256
wxyz: 4/256

also perfermance improve, in newer (>=1.19) go version, rand.Reader is not buffered any more, so it's suggested to wrap rand.Reader with bufio if the data read from reader is small.

https://tip.golang.org/doc/go1.19#:~:text=Read%20no%20longer%20buffers%20random%20data%20obtained%20from%20the%20operating%20system%20between%20calls

[trim21]

bufio.Reader is not thread-safe and sync.Pool perf better than mutex.

[codecov]

Codecov Report

Patch coverage: 90.47% and project coverage change: +0.03 🎉

Comparison is base (626f13e) 92.77% compared to head (479045b) 92.81%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2492      +/-   ##
==========================================
+ Coverage   92.77%   92.81%   +0.03%     
==========================================
  Files          39       39              
  Lines        4635     4646      +11     
==========================================
+ Hits         4300     4312      +12     
+ Misses        243      242       -1     
  Partials       92       92              

Impacted Files	Coverage Δ
middleware/util.go	90.76% <90.47%> (+3.73%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[aldas]

allright, people like me should not do security related things. @trim21 did not explain what we did before. Previously we had charset[rand.Int63()%int64(len(charset))] which was changed to charset[b%byte(len(charset))] that b is in range of 256 as @trim21 pointed out and moduling it with 52 (len(charset)) does not create equal possibilities.

Thank you @trim21

[aldas]

@trim21 would you like to add another PR that adds (doc)comment to randomString function that explains a little bit (more in depth) this bias when modulating rand.Int63() vs random byte. I think it would be beneficial for people who happen to skim over these parts or decide to copy it. I mean, I can not be only dummy in the world :)

PRs and their threads on Github are detached from the code and read rarely but library actual code is read more often - so it would be a great place to teach people not to be so security naive.

[trim21]

@trim21 would you like to add another PR that adds (doc)comment to randomString function that explains a little bit (more in depth) this bias when modulating rand.Int63() vs random byte. I think it would be beneficial for people who happen to skim over these parts or decide to copy it. I mean, I can not be only dummy in the world :)

PRs and their threads on Github are detached from the code and read rarely but library actual code is read more often - so it would be a great place to teach people not to be so security naive.

I'm not very good at English...