OSS-Fuzz integration

[guidovranken]

I'd like to integrate this library into OSS-Fuzz (https://github.com/google/oss-fuzz) for continuous fuzzing.

Only actively maintained projects can be integrated into OSS-Fuzz. Are you willing to patch any security issues found by OSS-Fuzz?

[kmackay]

Sure, sounds good.

…

On Nov 19, 2020, at 7:38 AM, Guido Vranken ***@***.***> wrote: I'd like to integrate this library into OSS-Fuzz (https://github.com/google/oss-fuzz <https://github.com/google/oss-fuzz>) for continuous fuzzing. Only actively maintained projects can be integrated into OSS-Fuzz. Are you willing to patch any security issues found by OSS-Fuzz? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#179>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYPM37AFIMYUDISYWYPMDLSQU3YDANCNFSM4T3R5EXQ>.

[guidovranken]

Excellent!

This is the first bug I found. The parameters in the following code passes ECDSA verification in other libraries like Botan, but fail in micro-ecc. Would you mind taking a look? Thanks!

C++:

#include <boost/multiprecision/cpp_int.hpp>
#include <stdio.h>
#include <string.h>
#include <cstdint>
#include <vector>
#include <string>

extern "C" {
    #include <uECC.h>
}

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }

static bool EncodeBignum(const std::string s, uint8_t* out, const size_t maxSize) {
    std::vector<uint8_t> v;
    boost::multiprecision::cpp_int c(s);
    boost::multiprecision::export_bits(c, std::back_inserter(v), 8);
    if ( v.size() > maxSize ) {
        return false;
    }
    const auto diff = maxSize - v.size();

    memset(out, 0, maxSize);
    memcpy(out + diff, v.data(), v.size());

    return true;
}

int main(void)
{
    const uint8_t in[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
    uECC_Curve curve = uECC_secp224r1();
    std::vector<uint8_t> pub, sig(64);

    pub.resize(uECC_curve_public_key_size(curve));

    CF_CHECK_EQ(EncodeBignum("23852691330394628764103282461488620235089260910792244717642832754951", sig.data(), 32), true);
    CF_CHECK_EQ(EncodeBignum("569869502696968957477203543617286211294873325770253161741945632088", sig.data() + 32, 32), true);
    CF_CHECK_EQ(EncodeBignum("25763354893102810033035439014968679320929073222431250678059382845957", pub.data(), pub.size() / 2), true);
    CF_CHECK_EQ(EncodeBignum("22333325989659214059802509033658178684218951124184979869072676254564", pub.data() + (pub.size() / 2), pub.size() / 2), true);

    printf("%d\n", uECC_verify(pub.data(), in, sizeof(in), sig.data(), curve));
end:
    return 0;
}

[kmackay]

It looks like your offsets are incorrect. For secp224r1, r and s are 28 octets long.

…

On Nov 21, 2020, at 11:56 PM, Guido Vranken ***@***.***> wrote: Excellent! This is the first bug I found. The parameters in the following code pass ECDSA verification in other libraries like Botan, but fail in micro-ecc. Would you mind taking a look? Thanks! C++: #include <boost/multiprecision/cpp_int.hpp> #include <stdio.h> #include <string.h> #include <cstdint> #include <vector> #include <string> extern "C" { #include <uECC.h> } #define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; } static bool EncodeBignum(const std::string s, uint8_t* out, const size_t maxSize) { std::vector<uint8_t> v; boost::multiprecision::cpp_int c(s); boost::multiprecision::export_bits(c, std::back_inserter(v), 8); if ( v.size() > maxSize ) { return false; } const auto diff = maxSize - v.size(); memset(out, 0, maxSize); memcpy(out + diff, v.data(), v.size()); return true; } int main(void) { const uint8_t in[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; uECC_Curve curve = uECC_secp224r1(); std::vector<uint8_t> pub, sig(64); pub.resize(uECC_curve_public_key_size(curve)); CF_CHECK_EQ(EncodeBignum("23852691330394628764103282461488620235089260910792244717642832754951", sig.data(), 32), true); CF_CHECK_EQ(EncodeBignum("569869502696968957477203543617286211294873325770253161741945632088", sig.data() + 32, 32), true); CF_CHECK_EQ(EncodeBignum("25763354893102810033035439014968679320929073222431250678059382845957", pub.data(), pub.size() / 2), true); CF_CHECK_EQ(EncodeBignum("22333325989659214059802509033658178684218951124184979869072676254564", pub.data() + (pub.size() / 2), pub.size() / 2), true); printf("%d\n", uECC_verify(pub.data(), in, sizeof(in), sig.data(), curve)); end: return 0; } — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#179 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYPM35XQ2IHG2IGGNCWLLLSRC72RANCNFSM4T3R5EXQ>.

[guidovranken]

Thanks! I'm now using the correct point size for each curve, and verification now succeeds for the parameters in my example.

However here is another discrepancy. The following parameters verify in wolfCrypt and libsecp256k1, but fail verification in micro-ecc. This is secp256k1 so I believe 32 bytes is the correct size for R and S.

#include <boost/multiprecision/cpp_int.hpp>
#include <stdio.h>
#include <string.h>
#include <cstdint>
#include <vector>
#include <string>

extern "C" {
    #include <uECC.h>
}

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }

static bool EncodeBignum(const std::string s, uint8_t* out, const size_t maxSize) {
    std::vector<uint8_t> v;
    boost::multiprecision::cpp_int c(s);
    boost::multiprecision::export_bits(c, std::back_inserter(v), 8);
    if ( v.size() > maxSize ) {
        return false;
    }
    const auto diff = maxSize - v.size();

    memset(out, 0, maxSize);
    memcpy(out + diff, v.data(), v.size());

    return true;
}

int main(void)
{
    const uint8_t in[] =
            {0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
             0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11};
    uECC_Curve curve = uECC_secp256k1();
    std::vector<uint8_t> pub, sig(64);

    pub.resize(uECC_curve_public_key_size(curve));
    CF_CHECK_EQ(EncodeBignum("110618813224107091100351766566588261013518646361399424304146461958647130377927", sig.data(), 32), true);
    CF_CHECK_EQ(EncodeBignum("56528019055117870811188539769759161932852696818058491284544029456598522370972", sig.data() + 32, 32), true);
    CF_CHECK_EQ(EncodeBignum("55066263022277343669578718895168534326250603453777594175500187360389116729240", pub.data(), pub.size() / 2), true);
    CF_CHECK_EQ(EncodeBignum("83121579216557378445487899878180864668798711284981320763518679672151497189239", pub.data() + (pub.size() / 2), pub.size() / 2), true);

    printf("%d\n", uECC_verify(pub.data(), in, sizeof(in), sig.data(), curve));
end:
    return 0;
}

[guidovranken]

I assume there is no interest.

[kmackay]

I don’t know what’s up with your specific test vector. I added an ECDSA test vector test, with values from https://crypto.stackexchange.com/questions/41316/complete-set-of-test-vectors-for-ecdsa-secp256k1 <https://crypto.stackexchange.com/questions/41316/complete-set-of-test-vectors-for-ecdsa-secp256k1> Feel free to add test vectors to that if you like.

…

On Nov 26, 2020, at 4:28 PM, Guido Vranken ***@***.***> wrote: Thanks! I'm now using the correct point size for each curve, and verification now succeeds for the parameters in my example. However here is another discrepancy. The following parameters verify in wolfCrypt and libsecp256k1, but fail verification in micro-ecc. This is secp256k1 so I believe 32 bytes is the correct size for R and S. #include <boost/multiprecision/cpp_int.hpp> #include <stdio.h> #include <string.h> #include <cstdint> #include <vector> #include <string> extern "C" { #include <uECC.h> } #define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; } static bool EncodeBignum(const std::string s, uint8_t* out, const size_t maxSize) { std::vector<uint8_t> v; boost::multiprecision::cpp_int c(s); boost::multiprecision::export_bits(c, std::back_inserter(v), 8); if ( v.size() > maxSize ) { return false; } const auto diff = maxSize - v.size(); memset(out, 0, maxSize); memcpy(out + diff, v.data(), v.size()); return true; } int main(void) { const uint8_t in[] = {0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11}; uECC_Curve curve = uECC_secp256k1(); std::vector<uint8_t> pub, sig(64); pub.resize(uECC_curve_public_key_size(curve)); CF_CHECK_EQ(EncodeBignum("110618813224107091100351766566588261013518646361399424304146461958647130377927", sig.data(), 32), true); CF_CHECK_EQ(EncodeBignum("56528019055117870811188539769759161932852696818058491284544029456598522370972", sig.data() + 32, 32), true); CF_CHECK_EQ(EncodeBignum("55066263022277343669578718895168534326250603453777594175500187360389116729240", pub.data(), pub.size() / 2), true); CF_CHECK_EQ(EncodeBignum("83121579216557378445487899878180864668798711284981320763518679672151497189239", pub.data() + (pub.size() / 2), pub.size() / 2), true); printf("%d\n", uECC_verify(pub.data(), in, sizeof(in), sig.data(), curve)); end: return 0; } — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#179 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYPM37L3TUIOAANTHEEZGTSR3XELANCNFSM4T3R5EXQ>.