Security: Don't put the same bytes.Buffer into sync.Pool twice

[lattwood]

This fixes #743, possibly #739.

Discovered this while tracking down an issue in https://github.com/linode/terraform-provider-linode.

The removed call to sync.Pool.Put is handled by the request body wrapper that puts the buffer back in the pool when the body is closed.

[lattwood]

This closes #739 as well.

Now that I’m thinking about it, this is a security issue, as it’s disclosing a HTTP body to an unrelated endpoints/servers.

[reedloden]

@jeevatkm Able to take a look here when you have a moment?

[jxsl13]

@jeevatkm pls merge

[safaci2000]

Any updates on this ticket?

[lattwood]

Your best bet is going to be forking this, or moving to a different library, based on how long this issue has been open.

[niksteff]

Yeah we are moving. Not only is the CVE a big problem, after the update to 2.10 we experienced fun bugs where the json request body of a previous or the same request (could not tell) was copied in the host part of the request URL. This occurred when multiple request were running concurrently and we changed nothing but the resty upgrade. Was fine for two years. I cannot prove it as I could not recreate the bug but a downgrade to 2.9.1 immediately solved the problem.

It could be a side effect of this buffer problem but as i cannot prove it i won't open a new thread. Just wanted to state it if somebody else experiences something different and has more time at hands to test it through.

Here is an example log from our kibana:

2023/11/28 13:51:16.736214 WARN RESTY parse "{"some very large json request body here"}/oms/customer/v3/users/2133478": first path segment in URL cannot contain colon, Attempt 1

Because of the WARN RESTY you can see the warning is logged from the resty code base as this log is hard coded there.

Here is my example gist where i could not recreate the problem and my time budget was done: https://gist.github.com/niksteff/2fd29672a24372782627d99ba2016031

[Coronon]

For everyone that needs a quick fix without downgrading to v2.9.1:

replace github.com/go-resty/resty/v2 => github.com/lattwood/resty/v2 v2.0.0-20231102200459-74e9e135ae0c

Put the above line into your go.mod file to override the resty code with this PR.

[vithubati]

Any timeline as to when this PR will be merged?

[jeevatkm]

@lattwood and members on the PR - Thanks for reaching out. I have been traveling on vacation days and will return from vacation in the first week of January.
I'm sorry for not checking my emails and notifications properly these days.

Let me merge this PR and make a release.

[jeevatkm]

@lattwood Thank you for the PR, appreciated.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (105f718) 96.51% compared to head (74e9e13) 96.51%.

Additional details and impacted files

@@            Coverage Diff             @@
##               v2     #745      +/-   ##
==========================================
- Coverage   96.51%   96.51%   -0.01%     
==========================================
  Files          12       12              
  Lines        1637     1636       -1     
==========================================
- Hits         1580     1579       -1     
  Misses         36       36              
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.