Skip to content
This repository was archived by the owner on Nov 10, 2025. It is now read-only.

Fix redirection#694

Closed
erikzhang wants to merge 5 commits intomasterfrom
fix-redirection
Closed

Fix redirection#694
erikzhang wants to merge 5 commits intomasterfrom
fix-redirection

Conversation

@erikzhang
Copy link
Member

Fix #693

Ashuaidehao and others added 4 commits December 15, 2021 10:27
* init

* upgrade neo

* useDiagnostic

* Update RpcServer.SmartContract.cs

* invoke tree

* add event

* Add storage changes

* Update nuget

* rename event state

* Unify

* update neofs

* fix json

Co-authored-by: Shargon <shargon@gmail.com>
Co-authored-by: Erik Zhang <erik@neo.org>
Co-authored-by: Owen Zhang <38493437+superboyiii@users.noreply.github.com>
* add in file copyright

* fix the copyright

* update copyright start year

* Delete copyright.sh

* Delete copyright.txt

Co-authored-by: Owen Zhang <38493437+superboyiii@users.noreply.github.com>
Co-authored-by: Erik Zhang <erik@neo.org>
* init

* refac

Co-authored-by: Shargon <shargon@gmail.com>
@erikzhang erikzhang requested a review from shargon March 2, 2022 03:34
@erikzhang erikzhang mentioned this pull request Mar 2, 2022
if (!Settings.Default.AllowedContentTypes.Contains(message.Content.Headers.ContentType.MediaType))
return (OracleResponseCode.ContentTypeNotSupported, null);

if (!Settings.Default.AllowPrivateHost && message.RequestMessage.RequestUri != uri)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it really works to prevent ssrf since the request is already sent in the line:

message = await client.GetAsync(uri, HttpCompletionOption.ResponseContentRead, cancellation);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It will leak information such as resource existence at least because this check is not the first branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @vang1ong7ang , I think that the redirection must be manual like #692

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, should not assume GET requests have no side effect.

@vang1ong7ang
Copy link

i prefer to avoid the request happening. not only the final content uploading.

@vang1ong7ang
Copy link

dns rebinding should be also considered

@dusmart
Copy link

dusmart commented Mar 2, 2022

dns rebinding should be also considered

It seems that it's difficult to prevent SSRF in .Net.
Only a doc for Python is found here in Chinese.

@vang1ong7ang
Copy link

@dusmart also in golang i think

@erikzhang
Copy link
Member Author

Since we support https only, I think we don't need to worry about dns rebinding.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

oracle service vulnerability: local information leak

5 participants