ambionics / phpggc

PHPGGC is a library of PHP unserialize() payloads along with a tool to generate them, from command line or programmatically.
https://ambionics.io/blog
Apache License 2.0
3.25k stars 502 forks source link

Introduce --session-encode/-se option #185

Closed Creastery closed 7 months ago

Creastery commented 7 months ago

Hey Charles,

I'm suggesting a new option, --session-encode/-se to allow generation of payloads using session_encode() instead of serialize().

The rationale behind introducing this option is documented in the changes as well:

This is useful if you have an existing arbitrary file write primitive, but the web root directory is non-writable. In such cases, it is possible to forge a session file containing an unserialize() payload and trigger the chain by visiting any webpage that invokes session_start().

Since introducing the --session-encode option as an enhancement, wrapper or encoder would require deserialising the payload and re-serialising it with session_encode(), I opted to implement this in PHPGGC::serialize().

cfreal commented 7 months ago

Hello Creastery,

I am still debating whether or not this is useful. On the bad side, it will always create a string such as _|<payload>, which does not bring much. On the other side, your code is super clean and not everybody knows that sessions are encoded this way in PHP.

My logic is the following:

Therefore, I feel like this does not help any of the two groups.

I'd be interested in anybody's opinion as to whether or not we need to add this feature.

jvoisin commented 7 months ago

Having the --session-encode option verbosely documented might help non-frequent users and provide a learning opportunity.

Creastery commented 7 months ago

Hello Charles,

Thanks for your feedback. I do agree and understand with your reasoning, since the usage of session_encode() payloads is rather niche.

For me though, I view this option as a small quality-of-life change, as this PR came about because of me repeatedly monkey-patching to switch between unserialize() and session_encode() more often than I would like to admit. :joy:

cfreal commented 7 months ago

@jvoisin makes a good point, and @swapgs seems to agree. This PR gets merged.

Thanks to you all!