JWT Fuzzer #6

preetkaran20 · 2020-05-30T21:40:42Z

This is the part 2 of the JWT Addon where Fuzzer Capabilities are added for Fuzzing JWT token.

src/main/java/org/zaproxy/zap/extension/jwt/utils/JWTUtils.java

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/ExtensionJWTFuzzer.java

preetkaran20 · 2020-06-01T14:55:00Z

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/ExtensionJWTFuzzer.java

+        ExtensionScript extensionScript =
+                Control.getSingleton().getExtensionLoader().getExtension(ExtensionScript.class);
+        HttpPanelManager panelManager = HttpPanelManager.getInstance();
+        panelManager.addRequestViewFactory(


This also adds JWT view to Http Fuzzer. Not sure if there is any way to handle this. Need to look back.
If we don't handle this then in Http Fuzzer it will look similar to JWT Fuzzer but working differs a lot wherein JWT fuzzer does many other things like Signing the token and chaning a field inside the token but http fuzzer just replaces the entire token @kingthorin @thc202 please suggest.

This is a good point, will check.

This might not be an issue as same menu item is now used for HttpFuzzer and JWT fuzzer

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/JWTFuzzAttackPopupMenuItem.java

src/main/java/org/zaproxy/zap/extension/jwt/JWTConfiguration.java

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/ExtensionJWTFuzzer.java

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/JWTFuzzAttackPopupMenuItem.java

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/JWTFuzzerHandler.java

preetkaran20 · 2020-06-01T21:58:34Z

Added PR to reduce JWTFuzzerHandler code that is copied from HttpFuzzer.
zaproxy/zap-extensions#2421

...n/java/org/zaproxy/zap/extension/jwt/fuzzer/messagelocations/JWTMessageLocationReplacer.java

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/JWTFuzzAttackPopupMenuItem.java

preetkaran20 · 2020-06-06T14:31:02Z

Hi @kingthorin and @thc202 ,

Following is the view of fuzzer:

I was in impression that in case of multiple message location each one message location replacer only modifies one http message at a time i.e. say if 2 message locations with one one value is there then 2 http messages will be generated and hence I introduced a field for specifying signature operation which is no signature, generate new signature and use old signature only. But as this is not the case hence this UI becomes invalid so i need to have this field applicable for all the message locations and hence it cannot be represent in message location so i was thinking of adding this field either in Options panel or in fuzzer policy. please suggest.

thanks,
Karan

preetkaran20 · 2020-06-06T14:33:06Z

Hi @kingthorin and @thc202 ,

As i have already mentioned that Http view now contains the JWT view and it looks like this:

I am not sure if it is ok or we want to remove it and how can we remove it.

thanks,
Karan

preetkaran20 · 2020-08-30T11:38:09Z

Seems fine to me. Still seems to be plenty if comments to yourself that are un-resolved though. Not sure how important those are/were.

i think they are outdated. However i will look at them once again for confirmation.

preetkaran20 · 2020-09-01T01:52:20Z

Seems fine to me. Still seems to be plenty if comments to yourself that are un-resolved though. Not sure how important those are/were.

Yes i checked and they are outdated. One was regarding testing RSA, I have checked again and it is working fine.
Just waiting for @thc202 's review comments, so shall i merge these changes and track them as bugs (if it takes time) or shall i wait for some more days ? @kingthorin what is your suggestion ?

thc202 · 2020-09-01T09:29:18Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

+    public void initView(ViewDelegate view) {
+        super.initView(view);
+
+        HttpPanelManager panelManager = HttpPanelManager.getInstance();


The Fuzzer add-on now allows to add views to just the the Fuzzer dialogue, through ExtensionFuzz#getClientMessagePanelManager and #getServerMessagePanelManager().

@thc202, I am not sure which one to use also i tried using clientMessagePanelManager and i am getting null pointer. I am not sure why. I am thinking that clientMessagePanelManager gets initialized in initview and hence may be that is causing issue.

It would be the "client" which in HTTP maps to "request". That's right, that would have to be done later in the hook method (if there's view), by then the view of the fuzz extension is already initialised.

thc202 · 2020-09-01T09:29:34Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

        try {
            extensionHook.addOptionsParamSet(getJWTConfiguration());
            extensionHook.getHookView().addOptionPanel(new JWTOptionsPanel());
+            ExtensionFuzz extensionFuzz =
+                    Control.getSingleton().getExtensionLoader().getExtension(ExtensionFuzz.class);
+            extensionFuzz.addFuzzerHandler(httpFuzzerHandler);
            LOGGER.debug("JWT Extension loaded successfully");
        } catch (Exception e) {
            LOGGER.error("JWT Extension can't be loaded. Configuration not found or invalid", e);


This should not be needed? (Core already catches exceptions that happen when hooking the extensions.)

thc202 · 2020-09-01T09:29:37Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

+    @Override
+    public void unload() {
+        super.unload();
+        HttpPanelManager panelManager = HttpPanelManager.getInstance();


For correctness the views need to be removed as well.

the views created by the factory ?

thc202 · 2020-09-01T09:30:35Z

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/ui/JWTFuzzPanelView.java

+                jwtComponentJsonKeysComboBox.setSelectedIndex(0);
+                jwtComponentType.addActionListener(getJWTComponentTypeActionListener(jwtHolder));
+            } catch (Exception e) {
+                LOGGER.error("Error Occurred: ", e);


It would be better if the errors were more descriptive, the users will reporting issues with this message. I'd also suggest using a lower level (e.g. warn) if this is expected to happen. Error level should be used just for potential bugs/issues in the code.

thc202 · 2020-09-01T09:39:04Z

Some comments to build.gradle.kts:

-addOnName.set("JWT Extension")
+addOnName.set("JWT Support") // Or something like that, add-ons are more than just extensions (e.g. this provides scan rules as well).

-implementation("org.zaproxy.addon:fuzz:13.0.0")
+compileOnly("org.zaproxy.addon:fuzz:13.0.0") // Might need testImplementation if you plan to add tests.

The dependencies block needs to have:

register("fuzz") {
    version.set("13.*")
}

thc202 · 2020-09-01T10:09:00Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

+        JWTI18n.init();
+        jwtMessageLocationReplacerFactory = new JWTMessageLocationReplacerFactory();
+        MessageLocationReplacers.getInstance()
+                .addReplacer(HttpMessage.class, jwtMessageLocationReplacerFactory);


Needs to be removed on #unload().

thc202 · 2020-09-01T10:10:05Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

        try {
            extensionHook.addOptionsParamSet(getJWTConfiguration());
            extensionHook.getHookView().addOptionPanel(new JWTOptionsPanel());
+            ExtensionFuzz extensionFuzz =
+                    Control.getSingleton().getExtensionLoader().getExtension(ExtensionFuzz.class);
+            extensionFuzz.addFuzzerHandler(httpFuzzerHandler);


thc202 · 2020-09-01T10:33:58Z

The changelog could be updated to mention the new functionality.

thc202 · 2020-09-01T12:48:18Z

Another comment for build.gradle.kts, under manifest it could have repo.set("https://github.com/SasanLabs/owasp-zap-jwt-addon/"), that's shown in the marketplace (site and ZAP).

preetkaran20 · 2020-09-01T13:09:02Z

Some comments to build.gradle.kts:

-addOnName.set("JWT Extension")
+addOnName.set("JWT Support") // Or something like that, add-ons are more than just extensions (e.g. this provides scan rules as well).

-implementation("org.zaproxy.addon:fuzz:13.0.0")
+compileOnly("org.zaproxy.addon:fuzz:13.0.0") // Might need testImplementation if you plan to add tests.

The dependencies block needs to have:

register("fuzz") {
    version.set("13.*")
}

what does register doing here ? i didn't got the use ? can you please explain.

thc202 · 2020-09-01T13:18:51Z

Sorry, that was not clear, that's for the dependencies under the manifest, to declare that the JWT add-on depends on the Fuzzer add-on (and that range version). Like was done for the Common Library.

thc202 · 2020-09-02T16:47:09Z

The add-on is not filling the "changes" entry of the manifest, since the add-on has a changelog you can add:

changesFile.set(tasks.named<ConvertMarkdownToHtml>("generateManifestChanges").flatMap { it.html })

to the manifest in build.gradle.kts. That's shown to the user to inform about the changes of the version installed or available in the marketplace.

preetkaran20 · 2020-09-03T01:35:53Z

@thc202 while installing and working on new fuzz version i am facing null pointers while saving options panel:

kingthorin · 2020-09-03T02:25:18Z

The work around is referenced here: zaproxy/zaproxy#6136

preetkaran20 · 2020-09-03T03:14:44Z

Hi @thc202 ,

I have incorporated changes suggested. Please review.

thanks,
Karan

thc202 · 2020-09-03T09:37:39Z

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to this add-on will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

 ## Unreleased
-
- First version of JWT Extension.
+- Second version of JWT Support.


This is a bit misleading, there's no first version (released) yet. Maybe call it iteration? Or just add all under first version?

thc202 · 2020-09-03T09:38:31Z

src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java

+                        .getExtensionLoader()
+                        .getExtension(ExtensionFuzz.class)
+                        .getClientMessagePanelManager();
+        panelManager.addViewFactory(RequestAllComponent.NAME, new JWTFuzzPanelViewFactory(null));


These (and the options panel) should be added only if there's view (there's a hasView() method), to avoid creating unnecessary view components in daemon and command line modes. Same check would apply in unload.

preetkaran20 · 2020-09-03T10:38:17Z

@thc202 done with the changes.

preetkaran20 · 2020-09-03T12:39:17Z

Hi @thc202 @kingthorin ,

please review zaproxy/zaproxy-website#101 also.

thanks,
Karan

thc202 · 2020-09-03T12:42:38Z

Looks good, thank you (especially for the patience)!

preetkaran20 · 2020-09-03T12:44:03Z

Merging it now. Thanks @thc202 and @kingthorin

preetkaran20 · 2020-09-03T12:47:00Z

@thc202 @kingthorin what next needs to be done for releasing this Project.

thc202 · 2020-09-03T14:24:32Z

I think the following steps:

Update the changelog to reflect the new version, e.g.:

-## Unreleased
+## [1.0.0] - 2020-08-03
 
 - First version of JWT Support.
   - Contains scanning rules for basic JWT related vulnerabilities.
   - Contains JWT Fuzzer for fuzzing the JWT's present in the request.
+
+[1.0.0]: https://github.com/SasanLabs/owasp-zap-jwt-addon/releases/tag/v1.0.0

Tag (above example using v1.0.0) and push.
Build the add-on (with Java 8) and create a new release from the tag.

Once that's done we can release to the marketplace.

preetkaran20 · 2020-09-03T14:41:37Z

Hi @thc202 @kingthorin

i have added the release https://github.com/SasanLabs/owasp-zap-jwt-addon/releases/tag/v1.0.0

thanks,
Karan

kingthorin · 2020-09-03T14:46:38Z

It doesn't seem to have the actual add-on (*.zap) attached?

thc202 · 2020-09-03T14:57:23Z

Sorry for the typo in the date.

preetkaran20 · 2020-09-03T14:58:56Z

Added the ".zap" file too.

thc202 · 2020-09-03T15:01:10Z

The jar can be removed, that will not work in ZAP.

First Commit with Fuzzer Changes

7fbcdc8

preetkaran20 commented May 30, 2020

View reviewed changes

src/main/java/org/zaproxy/zap/extension/jwt/utils/JWTUtils.java Show resolved Hide resolved

First Commit with Fuzzer Changes

4ed662b

preetkaran20 commented May 30, 2020

View reviewed changes

src/main/java/org/zaproxy/zap/extension/jwt/fuzzer/ExtensionJWTFuzzer.java Outdated Show resolved Hide resolved

preetkaran20 added 2 commits May 31, 2020 03:46

Fixing small issues

1b5edf0

Fixing small issues

e8fb42f