SasanLabs / owasp-zap-fileupload-addon

OWASP ZAP add-on for finding vulnerabilities in File Upload functionality.
Apache License 2.0
22 stars 6 forks source link

Missing Rule Details #17

Closed kingthorin closed 2 years ago

kingthorin commented 2 years ago

Describe the bug As reported here, the File Upload scan rule has no name for display in the policy manager and progress dialog.

To Reproduce Steps to reproduce the behavior:

Expected behavior Expect for all attacks to have a title so one knows what attack vector is used. Also for debugging if something goes wrong with this specific attack.

Screenshots image

and

image

Desktop (please complete the following information): [id=fileupload, version=1.1.0],

Additional context Can also be reproduced with: https://github.com/zaproxy/community-scripts/blob/main/standalone/Active%20scan%20rule%20list.js

kingthorin commented 2 years ago

@preetkaran20 we've figure out that this happens when the add-on is disabled via Options > Extensions.

The i18n handling is only initialized if the extension is enabled: https://github.com/SasanLabs/owasp-zap-fileupload-addon/blob/5d9fd7dd235f85dd79054ba6438e83d42fa513dc/src/main/java/org/sasanlabs/fileupload/ExtensionFileUpload.java#L38-L39

I'm happy to submit a fix moving that to a static block:

static{
    FileUploadI18n.init();
}

I just need you to confirm that the Scan Rule with work properly with the extension disabled. It seems like the extension is only responsible for hooking the options/params, so as long as they default appropriately it should be fine.

Edit: Looks like the Extension class could also use a getUIName() to display something more user friendly in the Options / Extensions screen.

kingthorin commented 2 years ago

This also seems to happen on subsequent starts of ZAP (even when the extension is enabled). Moving the i18n init to a static block seems to address the issue.

kingthorin commented 2 years ago

I just need you to confirm that the Scan Rule with work properly with the extension disabled. It seems like the extension is only responsible for hooking the options/params, so as long as they default appropriately it should be fine.

Poke @preetkaran20 :grinning:

preetkaran20 commented 2 years ago

@kingthorin Sorry for the late reply, I was confused and later forget about it.

Are we not facing the same issue in jwt add-on? Then we can initialize in init function similar to jwt add-on https://github.com/SasanLabs/owasp-zap-jwt-addon/blob/master/src/main/java/org/zaproxy/zap/extension/jwt/JWTExtension.java#L65

preetkaran20 commented 2 years ago

I am not sure how we can disable extension and what is the impact of that. Will it not call the hook method? If so, Then the option params will not get injected and scanrule will not execute any attack and just returns.

kingthorin commented 2 years ago

Yes it probably impacts the JWT add-on as well. (I can PR it too, if it'll work without the extension.) I didn't check it specifically. But I think thc202 mentioned the same.

If the extension is disabled then no the hook method won't be called.

I am not sure how we can disable extension and what is the impact of that.

Extensions/add-ons can be disabled via Tools > Options > Extensions.

If so, Then the option params will not get injected and scanrule will not execute any attack and just returns.

Okay that's fine then, I'll PR the change putting the i18n init in a static block.

preetkaran20 commented 2 years ago

I will tests the add-on from my side as well.