Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allow ASCII control character in URLs #790

Merged
merged 2 commits into from Apr 25, 2020
Merged

Conversation

erikdubbelboer
Copy link
Collaborator

@erikdubbelboer erikdubbelboer commented Apr 23, 2020

This is the same as net/http does.

Fixes #788

Copy link

@patryk4815 patryk4815 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is fixed now :)

@dfirsht
Copy link
Contributor

dfirsht commented Apr 24, 2020

Can you add tests for this? What will the response be if these characters are detected?

@erikdubbelboer
Copy link
Collaborator Author

I have added some tests. It's not possible at the point in the code to return an error so the only thing we can do is return an empty URL I'm afraid.

@erikdubbelboer erikdubbelboer merged commit 079f39b into master Apr 25, 2020
@erikdubbelboer erikdubbelboer deleted the ft/no-ctrl-in-uri branch April 25, 2020 18:55
@peczenyj
Copy link
Contributor

Hello,

I note, with this patch, if I try to parse a uri like this (after url decode):

const longURIEncoded = "http%3A%2F%2FDSeGOdaOceAcXFAcX.BPSHa.OSP%C3%A7bvMqDSppicaDPPpaJcq%C3%A7vbHI.aPIASJvepipFDSSbFDbaDNSvAWOD%C3%A0GPeM%40pbFiDOWvaeaenSZMSDpDJ%40IYSDW..vb..Y.c.XSXdePPPve.WNp%40vDSe%C3%A0YODADqa%C3%A8H%40cF%40eD%40nNdAFAP%40pb.aQOMOaadc%C3%A7pO%C3%A9eaMNIe.dMJPWc%C3%A0SiPanbNeP%40PJHaOd%40G.vSSv%40PDOqXv%40%C3%A9G%C3%A0pSPBODeDaHDPSBaPHnNbaYcOzdzDO%C3%A8Si%C3%A9.OvDP%C3%A0cZD..SO%C3%A9W.Q%C3%A9Pv.zNvqDGAHApP%C3%A0qpJDzHJcSbP%C3%A9eDOZPM%40OHO.DiOM%40Q%40eq.cnDvOOSvDQqPvW%C3%A0Q%C3%A8%40HDDA%C3%A9P..%40vpq.ezMMZqbc.na%40HII%C3%A8JPzWFPJiSaNPD%C3%A9.ePP%C3%A0O%C3%A8DOJGFJiP%C3%A9%C3%A8e%40DbPYa%40Ii%C3%A7X%40%C3%A8%C3%A0PbPDcS.SFDDDdNXDSSPJzDQ%C3%A9%C3%A7.n.%C3%A0cPDIabpO%C3%A8b%C3%A7v%C3%A7%C3%A9AGcBONDJe%C3%A7QQZiPJ.DDzvHvSWGvOSJeOaYaM%C3%A0eSPDbp%40DFcOpO%40.QiB%C3%A9cD.evpdez.%40%C3%A0p%40.%C3%A8OSN%40B%C3%A0NOvFSO%40PAce.%40%40.d%C3%A8Gv%C3%A9%40MD.DDPGJzDD%C3%A7N.OqcD%40DPDQXDOM%C3%A8iaJJpIJDQYbc%C3%A7SSavO%C3%A7P%C3%A7nIPWW%40BHaW%C3%A7aaHaOPDcWOSPSiSeb.SJ%40qAJzevQDWpOI.AS%C3%A9SA%40Fpi.XcQWDDPcOSbenOaqpcqdX%C3%A8JnzQAAaA%C3%A8%C3%A0bOe%40cbHPWp%C3%A0M%C3%A0%C3%A8n%40Dpc%C3%A8pep%40%40Op%C3%A8PMOPPIDacMDPIDDD%40vMSAMebq%40SDXOZi.ivO%40.DaQIbSSH%40nzSP.DD%C3%A9vOZ%40FQDpO.vIv%C3%A0A%40QbbpID%C3%A0epi%C3%A9dJ.DaDJMOcWF%C3%A9d%40qFJbcJZveMe%40YcpbQa%40pp%C3%A0JbJYSb%40.vF%40zYAcM%C3%A8DOAD%C3%A9AJq%40ScPOnP%C3%A0P%C3%A8YJ%C3%A9ccSN.BSccbSXX%C3%A0..GIDeeN%40pzSNNODDc%C3%A0YSeJv%C3%A0b%C3%A9OOJdeQ%40NHiDFzIY%C3%A8%C3%A7cNJa%C3%A8.YDcpnYWOi%40DPeDM.PYFWcJJPJ%C3%A9Pbza.Yn%C3%A9p%C3%A9.JpbPnF%40.PvzHMOQpv%C3%A7XnapqPnHJWqPGDbPccqdDOB.a%C3%A9P%C3%A0SPDJeSXBOMQa%C3%A8%C3%A7ZDaeNDDBNAPOBc%40ObSOOPb%C3%A8%C3%A7zdPnOJDcFzaqDOecn%40APSGJnvXGc%C3%A0PMDDAPPWzqdnp%C3%A0bPSnYF%C3%A8PD%C3%A7%C3%A9YP%C3%A8v.S.vZ%40.Yc%C3%A7OdnMS%C3%A0B%C3%A7..JPMO%C3%A9DvZDPezD%C3%A7ID.SpJGP.JJYDQOqSpbDvSXYDvZAMMD%C3%A8We%C3%A7Av%C3%A9%40%2F%3Fa%3D%27%26b%3D%22%26c%3D%3C%26d%3D%3E%7C%2B%29%C3%A0%C3%A7%22%C3%A9%27%C3%A7%C3%A8_-%28%27%C3%A9%22%26%2522%26%27%27%2Ftoto%C3%A7%3C%C3%A0%C3%A9*%404%C3%A9*%2B%7C4%3D%3F20%402%C3%A7%3E%3E%40%3C-%C3%A96%C3%A0%7C1%2B%2B381%C3%A72*9%C3%A9_08%2640%40%C3%A99%3D-%3C1%7C63%402%C3%A0%3F5%25%40%40-24%40%C3%A9%40%C3%A0%4095-6%3D%C3%A7%403%25%3D9%3D%40%255%2B%C3%A01%C3%A0%C3%A0%C3%A07*8%C3%A9%3F%40%3D%3D%25%3C%C3%A02%25_0%C3%A9%26%C3%A0%3F%C3%A9822%C3%A08%C3%A9%C3%A9%C3%A9%C3%A0%7C_%25-81-3%40%C3%A06%40_06%7C1%252%C3%A07%40%C3%A94051*%C3%A9%3E%3F9%3D-%C3%A9%405%40%40%C3%A734%2F%3D%40%3E%25%2B%C3%A7%3D%3D%C3%A7%25%2B%3C%3F%40%7C%3F%C3%A985%2B*_%2695%3F%25%25%C3%A90*%3E%C3%A7%C3%A9%C3%A9%C3%A0_4%405%7C97%C3%A9%C3%A06%C3%A92%3C%7C%C3%A9%2B-6%2F%C3%A916%C3%A01%40%26%C3%A0%25%C3%A9%3D%3E_%3C6%40%C3%A7%26%C3%A9%40%40%3D_%3C%C3%A0%7C%258%C3%A9--3*%26%7C9%3E1%2F0%C3%A9%255-7%C3%A74%26%25%400%3F1%C3%A0%3C0368%3D%C3%A969%C3%A7%C3%A04%406%40%2F%7C7%2F6%25%26-%2F%25%C3%A0%C3%A9%3F6%C3%A902%C3%A7-%C3%A9%40%4057%2B62%263%3E6-_%C3%A91990%2F7%C3%A7%C3%A0%2B-%2F%C3%A9%C3%A98%C3%A0%3D8%C3%A74*4%C3%A0%25-%C3%A9%259%3D04%C3%A05%C3%A0*%C3%A0114%3E%2F_1%7C1%40_2%7C%7C%256%3D%40%261%2535%3C%265%4057%7C%3C86_-3%3C62%26%C3%A93%2B48%260%3C-%C3%A9%C3%A0%3F%C3%A0--21%3C7%3F**3%7C%7C%26%40%C3%A7%C3%A9%3F3%40%26%7C-%C3%A90%C3%A9%3F_%26%C3%A0%2F-%C3%A9%252%3C%26%C3%A0*0%25%C3%A9%2B%3D%2B0%3F9%3E%3D%408%3C91%3E%C3%A9%26%3C%C3%A7%25%25%264%403%25-%250%C3%A0-%C3%A0%2F*%C3%A78_05%C3%A0%25%C3%A7%40%C3%A7-_-7_%3C%2F%3Fa%3D%27%26b%3D%22%26c%3D%3C%26d%3D%3E%7C%2B%29%C3%A0%C3%A7%22%C3%A9%27%C3%A7%C3%A8_-%28%27%C3%A9%22%26%2522%26%27%27%2Ftoto%26%C2%A5%26%0D%0A%26%2422%7C%7C%7C%3C%EF%98%83%3E%0D%0Awow%3C%EF%98%92%EF%80%80%3E+end" 

the full uri became http:/// and the only clue about this is an invalid uri is an empty host (since Parse does not return an error)

is it possible return something with size 0 in this case? Will be nice to know if the issue is "missing host" or "contain invalid chars"

Thanks

@erikdubbelboer
Copy link
Collaborator Author

@peczenyj would this work for you? #820

@peczenyj
Copy link
Contributor

peczenyj commented May 31, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request smuggling in fasthttpproxy.FasthttpHTTPDialer
4 participants