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

Fix condition in order to fix Firefox module #2217

Merged
merged 1 commit into from Aug 13, 2013

Conversation

jvazquez-r7
Copy link
Contributor

The actual condition is wrong, the not found should be sent if firefox isn't 17 AND firefox isn't 21, because the module supports Firefox 17 and 21 atm.

wchen-r7 added a commit that referenced this pull request Aug 13, 2013
@wchen-r7 wchen-r7 merged commit bc9a26d into rapid7:master Aug 13, 2013
@todb-r7
Copy link

todb-r7 commented Aug 13, 2013

Wait, I don't get this fix. Both seem to be okay. It's a little confusing because of the inverted match operator.

Seems like this would be clearer, but maybe I'm misunderstanding the check:

unless agent =~ /Firefox\/(17|21)/

This is an or as well, but I can't see how you could ever satisfy an and. When you invert the match, or and and are going to give the same results?

@rajchandel
Copy link

please send me exploited browser link

@jvazquez-r7
Copy link
Contributor Author

@todb-r7, at the moment Firefox 17 and 21 are supported so if the browser isn't Firefox 17 branch and isn't Firefox 21 branch send a Not Found because of the unsupported browser:

        if agent !~ /Firefox\/17/ and agent !~ /Firefox\/21/
            print_error("Browser not supported, sending 404: #{agent}")
            send_not_found(cli)
            return
        end

And, yes, your patch should also work:

unless agent =~ /Firefox\/(17|21)/
    print_error("Browser not supported, sending 404: #{agent}")
    send_not_found(cli)
    return
end

Since the ruby style guide says: "Favor unless over if for negative conditions (or control flow ||)." I'm going to make a new pull request with the @todb-r7 suggestion.

@todb
Copy link
Contributor

todb commented Aug 13, 2013

@rajchandel, see the answer to your request at #2198 (comment)

@wchen-r7
Copy link
Contributor

@jvennix-r7
Copy link
Contributor

It's too bad we can't distinguish between Firefox 17 and Firefox ESR 17. Technically we could do it with javascriptosdetect which uses feature detection instead of useragent parsing, but my guess is that javascriptosdetect would flag ESR firefox as a "falsely reported user agent" (since the feature set won't match up) and not return anything specific as its version.

@jvazquez-r7 jvazquez-r7 deleted the fix_firefox_condition branch November 18, 2014 15:47
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

Successfully merging this pull request may close these issues.

None yet

6 participants