OutOfMemoryError has been occurred

[hehexiansheng123]

My product tester is connected to a self-built email server. When verifying the email account or sending emails, a large packet is returned, causing memory overflow. Does this software verify the packet length in these two scenarios? If it is not verified, it is vulnerable to attacks.

Thanks

[hehexiansheng123]

According to the code, it seems that the response from the email server is not verified.

protected int readServerResponse() throws MessagingException {
assert Thread.holdsLock(this);
String serverResponse = "";
int returnCode = 0;
StringBuilder buf = new StringBuilder(100);

// read the server response line(s) and add them to the buffer
// that stores the response
    try {
    String line = null;

    do {
	line = lineInputStream.readLine();
	if (line == null) {
	    serverResponse = buf.toString();
	    if (serverResponse.length() == 0)
		serverResponse = "[EOF]";
	    lastServerResponse = serverResponse;
	    lastReturnCode = -1;
	    logger.log(Level.FINE, "EOF: {0}", serverResponse);
	    return -1;
	}
	buf.append(line);
	buf.append("\n");
    } while (isNotLastLine(line));

        serverResponse = buf.toString();
    } catch (IOException ioex) {
    logger.log(Level.FINE, "exception reading response", ioex);
        //ioex.printStackTrace(out);
    lastServerResponse = "";
    lastReturnCode = 0;
    throw new MessagingException("Exception reading response", ioex);
        //returnCode = -1;
    }

// print debug info
    //if (logger.isLoggable(Level.FINE))
        //logger.fine("RCVD: " + serverResponse);

// parse out the return code
    if (serverResponse.length() >= 3) {
        try {
            returnCode = Integer.parseInt(serverResponse.substring(0, 3));
        } catch (NumberFormatException nfe) {
	try {
	    close();
	} catch (MessagingException mex) {
	    // thrown by close()--ignore, will close() later anyway
	    logger.log(Level.FINE, "close failed", mex);
	}
	returnCode = -1;
        } catch (StringIndexOutOfBoundsException ex) {
	try {
	    close();
	} catch (MessagingException mex) {
	    // thrown by close()--ignore, will close() later anyway
	    logger.log(Level.FINE, "close failed", mex);
	}
            returnCode = -1;
    }
} else {
    returnCode = -1;
}
if (returnCode == -1)
    logger.log(Level.FINE, "bad server response: {0}", serverResponse);

    lastServerResponse = serverResponse;
lastReturnCode = returnCode;
    return returnCode;
}

[jbescos]

@hehexiansheng123 could you attach the stacktrace of the OutOfMemoryError, please?. I know that could happen in an unrelated area, but just in case it happens in the place we need to take a look...

[baoxinggit]

Hello, sorry to reply now,
this is the stack information:

overflow area:

[hehexiansheng123]

Hello, sorry to reply now, this is the stack information: overflow area:

@jbescos my colleagues had upload the stack information, take a look please, thanks bro

[jbescos]

Thank you both, @hehexiansheng123 and @baoxinggit

@jmehrens , I am thinking to provide a new property with a default value in LineInputStream to limit the bytes to be read.

Do you have any suggestion?.

[hehexiansheng123]

Thank you both, @hehexiansheng123 and @baoxinggit

@jmehrens , I am thinking to provide a new property with a default value in LineInputStream to limit the bytes to be read.

Do you have any suggestion?.

it seems a good idea, I think it's best to add a read length limit to the response body in the readLine method.

[jmehrens]

@jbescos

@jmehrens , I am thinking to provide a new property with a default value in LineInputStream to limit the bytes to be read.

Do you have any suggestion?.

I need to do some more work to provide better informed feedback. That said, I wonder if there is something documented in the RFCs about max length of a server response. If so we might be able to enforce a hard limit without a new property.

Adding a new property seems fine if no other alternative. Choosing right default might get tricky.

We should spend time thinking about adding enough namespace to the property to allow parent/child control of buffer sizes. E.G. all of Angus Mail, smtp, imap, pop3, and classname. Then decide if that provides value.

[jmehrens]

This thread talks a bit on limits:
jstedfast/MailKit#834

[baoxinggit]

Sorry, what is the plan for solving this problem?

[jmehrens]

@baoxinggit

Sorry, what is the plan for solving this problem?

Looks like there was a PR submitted for this issue. I linked it to this issue.
MailKit uses a 16k limit before failing and cites a RFC stating that there is a recommend limit of 1K and 8K for server responses but I wasn't able to locate the the RFC being cited.

I would think adding a new property with 16K limit default would be something to test out as I'm not sure what other details I'm missing on this change.

[baoxinggit]

May I ask when this issue can be merged and a new version released?

[jmehrens]

May I ask when this issue can be merged and a new version released?

Feedback was given in the PR no new changes were pushed. That is current blocker.

[baoxinggit]

Hi @jmehrens, just circling back on this issue as it's impacting our production environment.
Could you provide a rough timeline?
We understand maintainers have many priorities - any guidance would be appreciated.

[jmehrens]

@baoxinggit the linked PR is waiting on changes from @tbelaire who created the PR. Looks like the PR is abandoned.

This could move forward under a new PR with someone willing to work on it. I can review and shepherd but won't have time to author a new PR.