ericpaulbishop / gargoyle

Gargoyle Router Management Utility
http://www.gargoyle-router.com
465 stars 222 forks source link

Is it safe to use eval on user input? #969

Closed weakbytes closed 1 year ago

weakbytes commented 1 year ago

Almost all utility scripts contains following or similar:

eval $( gargoyle_session_validator -c "$POST_hash" -e "$COOKIE_exp" -a "$HTTP_USER_AGENT" -i "$REMOTE_ADDR" -r "login.sh" -t $(uci get gargoyle.global.session_timeout) -b "$COOKIE_browser_time"  )

Consider this: Let's assume: HTTP_USER_AGENT='$(ls > log.txt;sleep 30)' Then this is safe as nothing gets executed: echo $HTTP_USER_AGENT echo "$HTTP_USER_AGENT" But this is does gets executed: eval $(echo "$HTTP_USER_AGENT")

Do we have here potential RCE vulnerability using $POST_hash" "$COOKIE_exp" "$HTTP_USER_AGENT" "$COOKIE_browser_time"?

lantis1008 commented 1 year ago

Gargoyle is a single user system. You must have already broken the root password to be able to run any page that does this. If you've got root, you've got an easier attack route than this.

I get it, but I think the attack vector is kind of moot here. Do you agree? I'm happy to hear you out.

lantis1008 commented 1 year ago

Further, haserl filters these variables into the environment. So if there's a vulnerability it likely exists at that level. I recently tested this after an issue was raised against the diagnostics plug-in

weakbytes commented 1 year ago
  1. Thanks You for fast reply.
  2. Sometimes run_commands and/or get_password_cookie are running before login succeeds.
  3. it seems safe as long as gargoyle_session_validator does not "print" the user-input, otherwise it gets executed, 'echo' do prints.
  4. haserl even can translate from url encoded values giving more "obfuscation" to user-payload.
weakbytes commented 1 year ago

Further, haserl filters these variables into the environment. So if there's a vulnerability it likely exists at that level. I recently tested this after an issue was raised against the diagnostics plug-in

did You tried to inject both url encoded (full encoding and not full) data?

lantis1008 commented 1 year ago

I'll run some further validations. If you have a proof of concept vulnerability that you believe works, feel free to send it to me privately via the forum or you can email me. That way any potential vulnerability is disclosed privately and can be fixed before details are released.

weakbytes commented 1 year ago

I'll run some further validations. If you have a proof of concept vulnerability that you believe works, feel free to send it to me privately via the forum or you can email me. That way any potential vulnerability is disclosed privately and can be fixed before details are released.

Thank You,

weakbytes commented 1 year ago

Gargoyle router is no longer in my possession. But this part of code catch my eyes

lantis1008 commented 1 year ago

I was not able to produce any RCE type issues with this code. IF a line existed like eval $(echo "$HTTP_USER_AGENT") then yes it is very easy to trigger lots of unwanted behaviour. However passing it into the gargoyle_session_validator appears to do no harm, and I was not able to come up with any syntax that would escape out of this subshell and execute anything meaningful.

From my point of view, this issue should now be closed, and if in the future yourself (or anyone else) is able to produce some kind of vulnerability of this nature, please do let me know! Thanks for raising the issue!

lantis1008 commented 1 year ago

@weakbytes please close this issue

weakbytes commented 1 year ago

Closed