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

Demangler should create types referenced from demangled names that don't already exist #5920

Open
joelreymont opened this issue Sep 11, 2024 · 8 comments
Labels
Component: Demangler Effort: Medium Issue should take < 1 month Impact: Low Issue is a papercut or has a good, supported workaround Type: Enhancement Issue is a small enhancement to existing functionality

Comments

@joelreymont
Copy link

joelreymont commented Sep 11, 2024

Version and Platform (required):

  • Binary Ninja Version: 4.2.6042-dev, e875db7b
  • OS: macos
  • OS Version: 14.6
  • CPU Architecture: arm64

Internal binary name deploy article cage.

I have the following but it looks wrong as QString::fromUtf8 only takes 1 argument.

14004330a      QString::append(this: &arg_18, QString::fromUtf8(&arg_20, 0x40139b48))

So I 'Y' on QString::fromUtf8 and get

class QString `?fromUtf8@QString@@SA?AV1@PEBDH@Z`(char const*, int32_t)

I cannot remove the second argument as there's a problem with parsing the method name.

Is there a workaround?
Screenshot 2024-09-11 at 4 16 53 PM

@psifertex
Copy link
Member

deploy article cage

@psifertex
Copy link
Member

psifertex commented Sep 11, 2024

I think it does take a second argument according to https://doc.qt.io/qt-6/qstring.html#fromUtf8 (or rather, it can, so it isn't necessarily wrong) but I agree you should still be able to change it even if it's to something incorrect. 😉

I suspect the demangler is extracting types but since we don't have a QT type library it can't be applied. I'll let someone who knows the type system better confirm.

@joelreymont
Copy link
Author

Yes, there's a qsizetype size but are you suggesting it's 0x40139b48? 🤔

@joelreymont
Copy link
Author

This doesn't work either

QString `QString::fromUtf8`(const char* utf)

Screenshot 2024-09-11 at 4 32 45 PM

@psifertex
Copy link
Member

psifertex commented Sep 11, 2024

Change QString (the return value one specifically, not the one in quotes) -- as I said there's no type library so just put int or void* intead.

@psifertex
Copy link
Member

FWIW, both Ghidra and IDA show three parameters there actually, not one. So not sure which is correct, but probably not a single parameter.

@emesare
Copy link
Member

emesare commented Sep 11, 2024

Create a type called QString, then follow below.

Mark the type up with three parameters:

  1. Pre-allocated region for the QString, let's just call it this.
  2. The const char*
  3. The length, in most cases this will be -1 where the length of the const char* is taken.

struct QString `?fromUtf8@QString@@SA?AV1@PEBDH@Z`(struct QString* `this`, char const*, int32_t)

OR:

struct QString `QString::fromUtf8`(struct QString* `this`, char const*, int32_t)

ASIDE:

I would like to keep this issue open until someone can comment on the providence of debug/demangle info over our analysis of the function params.

i.e. in this case if we had not applied the type from demangled info we get the correct number of params.

@fuzyll
Copy link
Contributor

fuzyll commented Sep 11, 2024

Just in case this happens again in the future, if you hold your mouse over the error, it will show you the error message that came back from the parser:

image

So, in this case, it shows explicitly that it's having a problem parsing QString because it's referencing an unknown type.

I still think we should try and make these types (even if it's just a bare struct) so that hitting y Just Works, so I'm going to modify the name of the issue here and leave it open. But, since this'll likely come up again, now you should hopefully understand how to work around it for the time being.

@fuzyll fuzyll changed the title Unable to change the signature of C++ method: Name shown is not accepted Demangler should create types referenced from demangled names that don't already exist Sep 11, 2024
@fuzyll fuzyll added Type: Enhancement Issue is a small enhancement to existing functionality Component: Demangler Impact: Low Issue is a papercut or has a good, supported workaround Effort: Medium Issue should take < 1 month labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Demangler Effort: Medium Issue should take < 1 month Impact: Low Issue is a papercut or has a good, supported workaround Type: Enhancement Issue is a small enhancement to existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants