Closed hh-hunter closed 1 month ago
Hey @hh-hunter, thanks for your contribution!
I've reviewed your plugin and, although it works, I think there's some confusion here about the linked CVE.
I've done some research and, from what I could gather, it looks like CVE-2020-13945 only affects Apache APISIX v1.2 to v1.5 and has a CVSS 3.1 score of 6.5 as the affected versions do not allow to get RCE, as the script feature was introduced later, in APISIX v2. Moreover, the description of the CVE states that an attacker can interact with the API and access internal data if the default IP restriction rules of the admin API are deleted from the configuration file and the default API key is not changed.
Note that the security testbed you submitted targets APISIX v2.11, therefore it's definitely not vulnerable to CVE-2020-13945. Instead, to me it looks like it's a simple instance of an exposed admin interface using the default API key, which this plugin detects correctly. Also, I noticed that the linked issue (#217) actually reports the correct vulnerability (exposed interface with default secret) and there's no mention of the CVE.
All in all, I think this plugin works fine and is useful, but you should rename it appropriately (e.g. "Apache APISIX with default Admin token" or similar) and remove all the references to CVE-2020-13945.
I've also added some comments in the code with some small improvements that can be done. Moreover, please also implement the following changes:
- Before submitting, please run all the java files by the google-java-format tool
- Remove all the references to the CVE from the testbed PR too
- In the testbed files, please also add an example configuration for a non-vulnerable instance (e.g. set a different admin API key).
Ok, this PRP was submitted a long time ago, but the implemented code was written a while ago, I will remove all the information about CVE-2020-13945 as you suggested, thank you very much for your suggestion.
Hey, I have fixed these problems, please review @lokiuox
please review @lokiuox ,Ignore the word commit that I wrote incorrectly. Org
Hey, how to get the payload string generated by FakePayloadGeneratorModule in test?
Hey, how to get the payload string generated by FakePayloadGeneratorModule in test?
Sorry, I thought I added it to the review but must have forgotten to save the comment. With the random number generator always returning 0xFF
, the string will be this one:
private final static String DETECTION_STRING = "TSUNAMI_PAYLOAD_STARTffffffffffffffffTSUNAMI_PAYLOAD_END";
You can just add this to the test file.
completed
LGTM - Approved @maoning we can merge this and https://github.com/google/security-testbeds/pull/80
Reviewer: Savio, Doyensec Plugin: Apache APISIX Default Admin Token Detector Feedback: The detection quality is good, it just lacked some fingerprinting steps. The security testbed is good, although the safe configuration was initially missing. The plugin contains everything that is necessary for a fast detection of the vulnerability, but the referenced CVE was wrong, as the plugin detected a different issue. The author was very responsive. Drawback: None.
@lokiuox please review
Hi @hh-hunter,
This PR is being reviewed internally and should be merged in a few days.
~tooryx
@tooryx ,hi this is issue #217 merge pull request,please check this. The range environment is in https://github.com/google/security-testbeds/pull/80 @tooryx Please review