Skip to content

NdrCString packing is broken #84

Closed
#86
Closed
NdrCString packing is broken#84
#86

Description

@Couskito
Couskito
opened on Oct 6, 2025

Hi!

Thanks for the awesome library, I really like it!

While testing some RPC calls i noticed that the marshalling of C strings using NdrCString.pack() seemed to break for some reason (I can only assume this is an artifact left from the Python 2 transition).

Here is a simple program that should trigger the bug (at least it did on my setup using Python 3.9.13 and Windows 11) :

from windows.rpc import ndr

print(ndr.NdrCString.pack("Hello\x00"))

This yields the following stack trace :

PS C:\Users\cousky\Desktop\testdir> python .\test.py
Traceback (most recent call last):
  File "C:\Users\cousky\Desktop\testdir\test.py", line 3, in <module>
    a = ndr.NdrCString.pack("Hello\x00")
  File "C:\Users\cousky\AppData\Local\Programs\Python\Python39\lib\site-packages\windows\rpc\ndr.py", line 195, in pack
    result += data
TypeError: can't concat str to bytes

After having a quick look at windows\rpc\ndr.py it seems that encoding data to bytes by replacing result += data with result += data.encode() on line 195 fixes the issue, I can open a PR with the change for you if you wish.

Have a nice day!

Activity

  1. hakril commented on Oct 10, 2025

    @hakril
    hakril
    on Oct 10, 2025
    Owner

    Hi, thank you for the issue.

    Indeed, allowing string in python 3 would be nice.
    I am not a fan of using data.encode() as it would mean utf-8 encoding. And I am not sure that the RPC runtime will like that.

    For example, using French or Japanese would give

    >>> print(ndr.NdrCString.pack("Héhé\x00"))
b'\x05\x00\x00\x00\x00\x00\x00\x00\x05\x00\x00\x00H\xc3\xa9h\xc3\xa9\x00P'
>>> print(ndr.NdrCString.pack("ハロー・ワールド"))
b'\t\x00\x00\x00\x00\x00\x00\x00\t\x00\x00\x00\xe3\x83\x8f\xe3\x83\xad\xe3\x83\xbc\xe3\x83\xbb\xe3\x83\xaf\xe3\x83\xbc\xe3\x83\xab\xe3\x83\x89\x00PPP'

    That does not seem nice for an FC_C_CSTRING.

    My first approach would be to enforce ascii for the sake of simplicity, as encoding may depend on the locals of the computer running the code.
    I will try to test some stuff with a custom RPC server and update the issue in the next few days.

  2. hakril
    added a commit that references this issue on Oct 15, 2025

    Merge pull request #86 from hakril/add_ndr_tests

    f920ff5
  3. hakril
    closed this as completedin #86on Oct 15, 2025

  4. Couskito commented on Oct 16, 2025

    @Couskito
    Couskito
    on Oct 16, 2025
    Author

    Hi,

    You're right, using ascii is definitely safer.

    Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions