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

Commit a9e6f1df authored by Ken Chen's avatar Ken Chen Committed by android-build-team Robot
Browse files

Fix OOB read in DNS resolver 

The remote server specifies resplen, the length of the response it
intends to send. anssiz represents the size of the destination buffer.
If the reported resplen is larger than the anssiz, the code correctly
only reads up to anssiz bytes, but returns resplen. so later functions
will access far out of bounds.

The fix ensures that the length of send_vc return does not exceed the
buffer size.

Bug: 161362564
Test: atest pass on HWAddressSanitizer build.
Merged-In: Id4b5df1be4652e4623847b0b0bad0af65b80fdd5
Change-Id: Id4b5df1be4652e4623847b0b0bad0af65b80fdd5
(cherry picked from commit cf6ee247)
(cherry picked from commit 5214c6be)
parent c966227c

res_send.cpp

+4 −1
Original line number Diff line number Diff line
@@ -829,6 +829,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
@@ -839,7 +842,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) {