trailofbits / uthenticode

A cross-platform library for verifying Authenticode signatures
https://trailofbits.github.io/uthenticode/
MIT License
136 stars 33 forks source link

Small optimization for tohex #50

Closed hugmyndakassi closed 2 years ago

hugmyndakassi commented 2 years ago

Hi, I think you can do without a std::stringstream for a simple function like tohex() and still have a robust (RAII etc.) version.

Here's an alternative implementation in the context of a small test program:

#include <string>

static inline std::string tohex(std::uint8_t const* buf, std::size_t len)
{
    constexpr static char lookup_table[0x10] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};

    if (!buf || !len)
    {
        return {};
    }

    std::string hexstr;
    hexstr.reserve(len * 2); // each byte creates two hex digits

    for (auto i = 0; i < len; i++)
    {
        hexstr += lookup_table[buf[i] >> 4];
        hexstr += lookup_table[buf[i] & 0xF];
    }

    return hexstr;
}

#include <iostream>

int main()
{
    constexpr std::uint8_t testbuf[] = { 0xBA, 0xDB, 0xAD, 0xBE, 0xEF, 0xCA, 0xCE, 0xFE, 0xE1, 0xDE, 0xAD };

    std::cout << tohex(testbuf, sizeof(testbuf)) << std::endl;

    return 0;
}

Please note that the main reason for including iostream late is to show that the only required header for the code above is string.

I hope I didn't miss anything, but I attempted to keep the properties of the old function. Previously you also required enough space to keep the full buffer, so this should be largely the same.

Obviously you could also create two lookup tables, one lowercase, one uppercase and alternate between them base on a bool argument, for example (if you prefer that flexibility). Personally I am used to viewing hex in all uppercase.

If you feel like the above (with or without the discussed alterations) is worth including in uthenticode, please respond in a comment and I'll send a PR.


I am attaching a small benchmark merely for the performance aspect. But I think the above method is both simpler to write (consider the iomanip stuff in the original code) and to read and faster as well.

Benchmark: tohex_bench.zip or clone from here

Output:

$ hyperfine ./old_tohex; hyperfine ./new_tohex
Benchmark 1: ./old_tohex
  Time (mean ± σ):      6.061 s ±  0.137 s    [User: 4.812 s, System: 1.248 s]
  Range (min … max):    5.885 s …  6.276 s    10 runs

Benchmark 1: ./new_tohex
  Time (mean ± σ):      1.672 s ±  0.009 s    [User: 0.672 s, System: 1.000 s]
  Range (min … max):    1.658 s …  1.690 s    10 runs

hyperfine can be installed with cargo install hyperfine if you have Rust installed. Otherwise use whatever other tool you normally use for the purpose.

We need no warmup, because we read from /dev/urandom and throw it away into /dev/null after converting with either of the tohex() variants.


As far as I am concerned the optimized variant comes under the Unlicense, i.e. it is placed into the public domain plus the disclaimer of the MIT license. So if you don't insist on a PR, simply copy it over at your discretion if you find it worthy of inclusion.

woodruffw commented 2 years ago

Works for me; thanks for optimizing this function (it was indeed a very lazy implementation).

All caps is perfectly fine. Would you mind opening a PR with your changes?