Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
weakbytes opened this issue Jan 26, 2023 · 10 comments
Closed

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

weakbytes opened this issue Jan 26, 2023 · 10 comments

Comments

@weakbytes
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

  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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

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

@lantis1008
Copy link
Contributor

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
Copy link
Contributor

@weakbytes please close this issue

@weakbytes
Copy link
Author

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants