Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 339d9ea4 authored by Ken Chen's avatar Ken Chen Committed by Automerger Merge Worker
Browse files

Fix OOB read in DNS resolver am: 5214c6be 

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/DnsResolver/+/12324904

Change-Id: I54a45faac21e4bf03c66651c192fef3eff074f4b
parents 1f648647 5214c6be

res_send.cpp

+4 −1
Original line number Diff line number Diff line
@@ -838,6 +838,9 @@ read_len: 
            else

                break;

        }

        LOG(WARNING) << __func__ << ": resplen " << resplen << " exceeds buf size " << anssiz;

        // return size should never exceed container size

        resplen = anssiz;

    }

    /*

     * If the calling application has bailed out of
@@ -848,7 +851,7 @@ read_len: 
     */

    if (hp->id != anhp->id) {

        LOG(DEBUG) << __func__ << ": ld answer (unexpected):";

        res_pquery(ans, (resplen > anssiz) ? anssiz : resplen);

        res_pquery(ans, resplen);

        goto read_len;

    }

resolv_unit_test.cpp

+39 −3
Original line number Diff line number Diff line
@@ -123,9 +123,7 @@ class TestBase : public ::testing::Test { 
        dns.clearQueries();

    }



    int SetResolvers() {

        return resolv_set_nameservers(TEST_NETID, servers, domains, params);

    }

    int SetResolvers() { return resolv_set_nameservers(TEST_NETID, servers, domains, params); }



    const android_net_context mNetcontext = {

            .app_netid = TEST_NETID,
@@ -1139,6 +1137,44 @@ TEST_F(ResolvGetAddrInfoTest, TruncatedResponse) { 
    }

}



// Audit if Resolver read out of bounds, which needs HWAddressSanitizer build to trigger SIGABRT.

TEST_F(ResolvGetAddrInfoTest, OverlengthResp) {

    std::vector<std::string> nameList;

    // Construct a long enough record that exceeds 8192 bytes (the maximum buffer size):

    // Header: (Transaction ID, Flags, ...)                                        = 12   bytes

    // Query: 19(Name)+2(Type)+2(Class)                                            = 23   bytes

    // The 1st answer RR: 19(Name)+2(Type)+2(Class)+4(TTL)+2(Len)+77(CNAME)        = 106  bytes

    // 2nd-50th answer RRs: 49*(77(Name)+2(Type)+2(Class)+4(TTL)+2(Len)+77(CNAME)) = 8036 bytes

    // The last answer RR: 77(Name)+2(Type)+2(Class)+4(TTL)+2(Len)+4(Address)      = 91   bytes

    // ----------------------------------------------------------------------------------------

    // Sum:                                                                          8268 bytes

    for (int i = 0; i < 10; i++) {

        std::string domain(kMaxmiumLabelSize / 2, 'a' + i);

        for (int j = 0; j < 5; j++) {

            nameList.push_back(domain + std::string(kMaxmiumLabelSize / 2 + 1, '0' + j) +

                               kExampleComDomain + ".");

        }

    }

    test::DNSResponder dns;

    dns.addMapping(kHelloExampleCom, ns_type::ns_t_cname, nameList[0]);

    for (size_t i = 0; i < nameList.size() - 1; i++) {

        dns.addMapping(nameList[i], ns_type::ns_t_cname, nameList[i + 1]);

    }

    dns.addMapping(nameList[nameList.size() - 1], ns_type::ns_t_a, kHelloExampleComAddrV4);



    ASSERT_TRUE(dns.startServer());

    ASSERT_EQ(0, SetResolvers());

    addrinfo* result = nullptr;

    const addrinfo hints = {.ai_family = AF_INET};

    NetworkDnsEventReported event;

    int rv = resolv_getaddrinfo("hello", nullptr, &hints, &mNetcontext, &result, &event);

    ScopedAddrinfo result_cleanup(result);

    EXPECT_EQ(rv, EAI_FAIL);

    EXPECT_TRUE(result == nullptr);

    EXPECT_EQ(GetNumQueriesForProtocol(dns, IPPROTO_UDP, kHelloExampleCom), 2U);

    EXPECT_EQ(GetNumQueriesForProtocol(dns, IPPROTO_TCP, kHelloExampleCom), 2U);

}



TEST_F(GetHostByNameForNetContextTest, AlphabeticalHostname) {

    constexpr char host_name[] = "jiababuei.example.com.";

    constexpr char v4addr[] = "1.2.3.4";

tests/dns_responder/dns_responder.cpp

+6 −6
Original line number Diff line number Diff line
@@ -252,7 +252,7 @@ char* DNSQuestion::write(char* buffer, const char* buffer_end) const { 
}



std::string DNSQuestion::toString() const {

    char buffer[4096];

    char buffer[16384];

    int len = snprintf(buffer, sizeof(buffer), "Q<%s,%s,%s>", qname.name.c_str(),

                       dnstype2str(qtype), dnsclass2str(qclass));

    return std::string(buffer, len);
@@ -291,7 +291,7 @@ char* DNSRecord::write(char* buffer, const char* buffer_end) const { 
}



std::string DNSRecord::toString() const {

    char buffer[4096];

    char buffer[16384];

    int len = snprintf(buffer, sizeof(buffer), "R<%s,%s,%s>", name.name.c_str(), dnstype2str(rtype),

                       dnsclass2str(rclass));

    return std::string(buffer, len);
@@ -418,7 +418,7 @@ char* DNSHeader::write(char* buffer, const char* buffer_end) const { 


// TODO: convert all callers to this interface, then delete the old one.

bool DNSHeader::write(std::vector<uint8_t>* out) const {

    char buffer[4096];

    char buffer[16384];

    char* end = this->write(buffer, buffer + sizeof buffer);

    if (end == nullptr) return false;

    out->insert(out->end(), buffer, end);
@@ -896,7 +896,7 @@ bool DNSResponder::makeTruncatedResponse(DNSHeader* header, char* response, 


bool DNSResponder::makeResponse(DNSHeader* header, int protocol, char* response,

                                size_t* response_len) const {

    char buffer[4096];

    char buffer[16384];

    size_t buffer_len = sizeof(buffer);

    bool ret;
@@ -1059,7 +1059,7 @@ bool DNSResponder::addFd(int fd, uint32_t events) { 
}



void DNSResponder::handleQuery(int protocol) {

    char buffer[4096];

    char buffer[16384];

    sockaddr_storage sa;

    socklen_t sa_len = sizeof(sa);

    ssize_t len = 0;
@@ -1103,7 +1103,7 @@ void DNSResponder::handleQuery(int protocol) { 
    }

    LOG(DEBUG) << "read " << len << " bytes on " << dnsproto2str(protocol);

    std::lock_guard lock(cv_mutex_);

    char response[4096];

    char response[16384];

    size_t response_len = sizeof(response);

    // TODO: check whether sending malformed packets to DnsResponder

    if (handleDNSRequest(buffer, len, protocol, response, &response_len) && response_len > 0) {