Skip to content

Commit 25a2da3

Browse files
committed
Fix null pointer bug for IP packet headers
Sessions where registered before they were fully populated. In some cases, that could plausibly allow them to be handled by the NIO thread before their lastIP/TCP header values were set. In that case, if a response was sufficiently fast (e.g. there was an immediate error) then the NIO thread crashed trying to get the last IP header details of the session to write a response. We now ensure everything is 100% set up before we register the NIO channel.
1 parent 070830e commit 25a2da3

File tree

3 files changed

+12
-10
lines changed

3 files changed

+12
-10
lines changed

app/src/main/java/tech/httptoolkit/android/ProxyVpnRunnable.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class ProxyVpnRunnable(
3838
private val nioService = SocketNIODataService(vpnPacketWriter)
3939
private val dataServiceThread = Thread(nioService, "Socket NIO thread")
4040

41-
private val manager = SessionManager(nioService)
41+
private val manager = SessionManager()
4242
private val handler = SessionHandler(manager, nioService, vpnPacketWriter)
4343

4444
// Allocate the buffer for a single packet.

app/src/main/java/tech/httptoolkit/android/vpn/SessionHandler.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ private void handleUDPPacket(ByteBuffer clientPacketData, IPv4Header ipHeader) t
102102
ipHeader.getSourceIP(), udpheader.getSourcePort()
103103
);
104104

105+
boolean newSession = session != null;
106+
105107
if (session == null) {
106108
session = manager.createNewUDPSession(
107109
ipHeader.getDestinationIP(), udpheader.getDestinationPort(),
@@ -112,9 +114,12 @@ private void handleUDPPacket(ByteBuffer clientPacketData, IPv4Header ipHeader) t
112114
synchronized (session) {
113115
session.setLastIpHeader(ipHeader);
114116
session.setLastUdpHeader(udpheader);
115-
int len = manager.addClientData(clientPacketData, session);
117+
manager.addClientData(clientPacketData, session);
116118
session.setDataForSendingReady(true);
117119

120+
// We don't register the session until it's fully populated (as above)
121+
if (newSession) nioService.registerSession(session);
122+
118123
// Ping the NIO thread to write this, when the session is next writable
119124
session.subscribeKey(SelectionKey.OP_WRITE);
120125
nioService.refreshSelect(session);
@@ -380,6 +385,11 @@ private void replySynAck(IPv4Header ip, TCPHeader tcp) throws IOException {
380385
//client initial sequence has been incremented by 1 and set to ack
381386
session.setRecSequence(tcpheader.getAckNumber());
382387

388+
session.setLastIpHeader(ip);
389+
session.setLastTcpHeader(tcp);
390+
391+
nioService.registerSession(session);
392+
383393
writer.write(packet.getBuffer());
384394
Log.d(TAG,"Send SYN-ACK to client");
385395
}

app/src/main/java/tech/httptoolkit/android/vpn/SessionManager.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,6 @@ public class SessionManager implements ICloseSession {
5353
private final Map<String, Session> table = new ConcurrentHashMap<>();
5454
private SocketProtector protector = SocketProtector.getInstance();
5555

56-
private SocketNIODataService nioService;
57-
58-
public SessionManager(SocketNIODataService nioService) {
59-
this.nioService = nioService;
60-
}
61-
6256
/**
6357
* keep java garbage collector from collecting a session
6458
* @param session Session
@@ -159,7 +153,6 @@ public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) thr
159153
channel.connect(socketAddress);
160154
session.setConnected(channel.isConnected());
161155

162-
nioService.registerSession(session);
163156
table.put(keys, session);
164157

165158
Log.d(TAG,"new UDP session successfully created.");
@@ -205,7 +198,6 @@ public Session createNewTCPSession(int ip, int port, int srcIp, int srcPort) thr
205198
boolean connected = channel.connect(socketAddress);
206199
session.setConnected(connected);
207200

208-
nioService.registerSession(session);
209201
table.put(key, session);
210202

211203
return session;

0 commit comments

Comments
 (0)