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

Add / (search) commands to newshell + rewrite of rz_search #4742

Draft
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

wargio
Copy link
Member

@wargio wargio commented Nov 25, 2024

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

This is a complete rewrite of the legacy code which is too complicated to import into new-shell.

The goal is to refactor rz_search and implement the search command properly using pre-existing code like search of strings via rz_utils and more.

closes #1909

RZ_API void rz_search_collection_free(RZ_NULLABLE RzSearchCollection *sc);
RZ_API void rz_search_hit_free(RZ_NULLABLE RzSearchHit *hit);

RZ_API RZ_OWN RzList /*<RzSearchHit *>*/ *rz_search_run(RZ_NONNULL RzSearchOpt *opt, RZ_NONNULL RzSearchCollection *col, RZ_NONNULL RzIO *io, RZ_NONNULL RzList /*<RzIOMap *>*/ *search_in);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe changing the search_in parameter to void * and infer the type based on some flag in opt. This way we can use this function later to search other things then IO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure about this, but maybe.


struct rz_search_collection_t {
RzPVector /*<void *>*/ *collection; ///< Collection of elements to search in a buffer
RzSearchOverCallback search_over; ///< Collection search over the collection callback
Copy link
Member

@Rot127 Rot127 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs are a little confusing. Is the search_over callback called for every element in the collection above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the function takes the vector and uses it


typedef struct rz_search_hit_t {
RzSearchKeyword *kw;
ut64 addr;
char *metadata; ///< Metadata for extra details (can be NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to save mostly the type of the search hit or the things to search for (I saw aes and regex). Maybe changing it to an enum (RzSearchMetaDataType type) and a add a void *meta_data to save more complex stuff. This way we can save allocation of simple strings.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it for the flags so we know it's hit.regex or hit.rsa etc...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. What about hit_desc then? Meta data seems kinda broad IMHO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure for me.

@@ -3,8 +3,174 @@
---
name: cmd_search
commands:
- name: "//"
summary: Repeat the last seach
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
summary: Repeat the last seach
summary: Repeat the last search

@@ -3745,16 +3792,18 @@ RZ_API int rz_core_config_init(RzCore *core) {
SETOPTIONS(n, "auto", "rosections", "raw", NULL);

/* search */
SETCB("search.contiguous", "true", &cb_contiguous, "Accept contiguous/adjacent search hits");
SETICB("search.align", 0, &cb_searchalign, "Only catch aligned search hits");
SETB("search.progress", false, "Shows search progress (true: enable, false: disable)");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

*
* \return On success returns a valid pointer, otherwise NULL.
*/
RZ_IPI RZ_OWN RzSearchCollection *rz_search_collection_new(RZ_NONNULL RzSearchOverCallback search_over, RZ_NULLABLE RzPVectorFree free) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RZ_IPI RZ_OWN RzSearchCollection *rz_search_collection_new(RZ_NONNULL RzSearchOverCallback search_over, RZ_NULLABLE RzPVectorFree free) {
RZ_IPI RZ_OWN RzSearchCollection *rz_search_collection_new(RZ_NONNULL RzSearchOverCallback search_over, RZ_NULLABLE RzPVectorFree element_free) {

Or something similar.

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great work!

@@ -4371,6 +4371,36 @@ RZ_IPI RzCmdStatus rz_print_pattern8_handler(RzCore *core, int argc, const char
return RZ_CMD_STATUS_OK;
}

static void incAlphaBuffer(ut8 *buf, int bufsz) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are very weird functions

struct search_parameters {
RzCore *core;
typedef struct search_parameters {
RzCore *core; ///< RzCore instance to use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the search parameters should not store RzCore and RzCmdStateOutput, or maybe rename the structure in that case.

typedef struct search_prelude_cb_ctx {
int counter; ///< Number of preludes found
RzCore *core; ///< RzCore to use
int depth; ///< analysis.depth value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be size_t or ut32 for both depth and counter?

- RZ_OUTPUT_MODE_TABLE
args: []

- name: "/p"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For every search subcommand we need more detailed description and few examples.

@@ -15,6 +15,18 @@
#include <rz_io.h>
#include <rz_bin.h>

typedef enum {
SEARCH_MODE_BYTES = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be in librz/include/rz_search.h and also documented.

// SPDX-FileCopyrightText: 2008 J. Alex Halderman
// SPDX-License-Identifier: BSD-3-Clause

// Originally from RSAKeyFinder 1.0 (2008-07-18) By Nadia Heninger and J. Alex Halderman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, please convert into \file

{ 0x02, 0x01, 0x00, 0x30 },
};

/*Baby BER parser, just good enough for private keys.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is comment already for this function , maybe convert to Doxygen?

return start + 2 + lensize;
}

/* Check if `start` points to an ensemble of BER fields
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

return true;
}

// Finds and return index of a private key based on the asn1 bytes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


#include <rz_search.h>
#include <rz_util.h>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add \file to note that it searches not only ANSI/ASCII but also Unicode and some other encodings

@XVilka XVilka added this to the 0.8.0 milestone Nov 30, 2024
@XVilka XVilka added the blocker label Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants